Fix unfold2d and add test #443
Conversation
|
@alexandresablayrolles has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| W + 2 * padding[1] - (kernel_size[1] + (kernel_size[1] - 1) * (dilation[1] - 1)) | ||
| ) // stride[1] + 1 | ||
| input = F.pad(input, (padding[0], padding[0], padding[1], padding[1])) | ||
| # F.pad's first argument is the padding of the *last* dimension |
|
Please fix lint before merging. |
|
Looks great, thanks for the fix! Can we also add more inputs to |
|
@alexandresablayrolles has updated the pull request. You must reimport the pull request before landing. |
|
@alexandresablayrolles has updated the pull request. You must reimport the pull request before landing. |
karthikprasad
left a comment
There was a problem hiding this comment.
Thanks. LGTM. Please isort before merging.
|
@alexandresablayrolles has updated the pull request. You must reimport the pull request before landing. |
|
@alexandresablayrolles has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Fixes #442.
TL;DR: F.pad has an unintuitive syntax (we need to indicated padding from last dimensions to first dimensions).
This PR fixes that in unfold2d and adds tests with non-symmetric pad/dilation/kernel_size/stride.