Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pooling code does not allow sliding window starting in right padded region #46929

Open
heitorschueroff opened this issue Oct 27, 2020 · 0 comments
Open

Pooling code does not allow sliding window starting in right padded region #46929

heitorschueroff opened this issue Oct 27, 2020 · 0 comments
Labels
module: pooling triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@heitorschueroff
Copy link
Contributor

heitorschueroff commented Oct 27, 2020 •
edited

The current pooling code does not allow the sliding window to start inside the right padding when ceil_mode=True which is inconsistent with the equation on the docs. This PR #46558 adds a note to the documentation but does not address this issue because it requires further investigation into existing code assumptions.

The code that computes the output shape here does not include right padding on ceil_mode correction.

pytorch/aten/src/ATen/native/Pool.h

Line 34 in bbe5bfa

if ((outputSize - 1) * stride >= inputSize + pad_l)

cc @heitorschueroff
@heitorschueroff heitorschueroff mentioned this issue Oct 27, 2020
Closed
1 task
@mrshenli mrshenli added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Oct 27, 2020
heitorschueroff added a commit that referenced this issue Oct 27, 2020
@heitorschueroff
Update on "Fix max_pool2d with ceil_mode bug"
4009646 
This PR fixes a bug with how pooling output shape was computed.

## BC Breaking Notes
Previously, a bug in the pooling code allowed a sliding window to be entirely off bounds. Now, sliding windows must start inside the input or left padding (not right padding, see #46929) and may only go off-bounds if ceil_mode=True.

fixes #45357

TODO

- [x] Ensure existing tests are checking for the correct output size

[ghstack-poisoned]
heitorschueroff added a commit that referenced this issue Oct 29, 2020
@heitorschueroff
Update on "Fix max_pool2d with ceil_mode bug"
a4f0899 
This PR fixes a bug with how pooling output shape was computed.

## BC Breaking Notes
Previously, a bug in the pooling code allowed a sliding window to be entirely off bounds. Now, sliding windows must start inside the input or left padding (not right padding, see #46929) and may only go off-bounds if ceil_mode=True.

fixes #45357

TODO

- [x] Ensure existing tests are checking for the correct output size

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this issue Oct 30, 2020
@heitorschueroff @facebook-github-bot
Fix max_pool2d with ceil_mode bug (#46558)
2643800 
Summary:
Pull Request resolved: #46558

This PR fixes a bug with how pooling output shape was computed.

## BC Breaking Notes
Previously, a bug in the pooling code allowed a sliding window to be entirely off bounds. Now, sliding windows must start inside the input or left padding (not right padding, see #46929) and may only go off-bounds if ceil_mode=True.

fixes #45357

TODO

- [x] Ensure existing tests are checking for the correct output size

Test Plan: Imported from OSS

Reviewed By: albanD

Differential Revision: D24633372

Pulled By: heitorschueroff

fbshipit-source-id: 55925243a53df5d6131a1983076f11cab7516d6b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
module: pooling triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@heitorschueroff @mrshenli