Skip to content

Add unfold to autograd#1523

Merged
apaszke merged 4 commits intopytorch:masterfrom
fmassa:unfold_autograd
May 11, 2017
Merged

Add unfold to autograd#1523
apaszke merged 4 commits intopytorch:masterfrom
fmassa:unfold_autograd

Conversation

@fmassa
Copy link
Member

@fmassa fmassa commented May 9, 2017

Unfold is very handy whenever we want to perform operations in local neighbourhoods (like in convolutions or poolings).

For example, 2d max-pooling can be alternatively computed as follows:

def max_pool2d(input, kernel_size, stride):
    kh, kw = kernel_size
    dh, dw = stride
    # get all image windows of size (kh, kw) and stride (dh, dw)
    input_windows = input.unfold(2, kh, dh).unfold(3, kw, dw)
    # view the windows as (kh * kw)
    input_windows = input_windows.contiguous().view(*input_windows.size()[:-2], -1)
    max_val, max_idx = input_windows.max(4)
    return max_val, max_idx

apaszke
apaszke approved these changes May 10, 2017
View reviewed changes
Copy link
Contributor

@apaszke apaszke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, backward will need a lot of memory for idx_unfolded, right?

@apaszke
Copy link
Contributor

apaszke commented May 10, 2017

Can you please fix the conflicts?

@fmassa fmassa mentioned this pull request May 11, 2017
Closed
@fmassa
Copy link
Member Author

fmassa commented May 11, 2017

I think the conflicts were fixed.
The amount of memory needed for idx_unfolded will be roughly input_numel * size / stride, so it's not a huge increase for small neighborhood sizes. But it is more memory hungry if applied several times (for 2d conv for example) than a dedicated implementation of im2col.

@apaszke
Copy link
Contributor

apaszke commented May 11, 2017

I'm wondering if it wouldn't be better to wait for the im2col kernels 😕

@fmassa
Copy link
Member Author

fmassa commented May 11, 2017

If we have a Nd im2col (which works on 1d, 2d and 3d data), then I'm fine with closing this PR.
In such cases, we should probably remove unfold from pytorch as well :p

@soumith
Copy link
Collaborator

soumith commented May 11, 2017

dude let's just merge this in. we can always remove it later. waiting for xxx where we don't know the timeline is not worth it

@apaszke
Copy link
Contributor

apaszke commented May 11, 2017

Ok. There are still some conflicts though

@apaszke apaszke merged commit be843eb into pytorch:master May 11, 2017
@fmassa fmassa deleted the unfold_autograd branch May 11, 2017 15:53
Jiaming-Liu pushed a commit to Jiaming-Liu/pytorch that referenced this pull request May 18, 2017
@fmassa @Jiaming-Liu
Add unfold to autograd (pytorch#1523)
3cbed4e
@fmassa fmassa mentioned this pull request Oct 31, 2017
Closed
jjsjann123 pushed a commit to jjsjann123/pytorch that referenced this pull request Mar 21, 2022
@naoyam
Fixes pytorch#1523 (pytorch#1525)
5ba9343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@apaszke apaszke apaszke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fmassa @apaszke @soumith