Skip to content

Arm backend: Fix ceil_mode pooling when the last window is dropped - #24893

Open
EylonKrause wants to merge 2 commits into
pytorch:mainfrom
EylonKrause:fix/pool-ceil-mode-dropped-window
Open

EylonKrause wants to merge 2 commits into
pytorch:mainfrom
EylonKrause:fix/pool-ceil-mode-dropped-window

Conversation

@EylonKrause

@EylonKrause EylonKrause commented Nov 11, 2026 •

Copy link
Copy Markdown

Summary

PyTorch drops a last ceil_mode pooling window that would start in the right padding, so e.g. max_pool2d(kernel=2, stride=4, padding=0, ceil_mode=True) on a width of 7 gives 2 output columns, the same as floor mode. The input then has a remainder that no window uses.

SizeAdjustInputPass skipped every ceil_mode pool, so that remainder was never sliced off and adjust_pooling_pad_if_needed returned a negative post padding (-1 above). MAX_POOL2D and AVG_POOL2D were then emitted with a negative pad, which the TOSA reference model rejects ("At least one pad is smaller than zero"). For avg_pool2d with count_include_pad=True, DecomposeAvgPool2dPass fell back to padding with the full symmetric pad instead, which kept the dropped window and produced one output element too many.

This change:

  • slices the unused input for ceil_mode pools with static spatial shapes in every dimension where ceil_mode does not add an output element (when the last window is dropped, the ceil and floor output sizes are equal, so the floor-mode slice is exact);
  • does not add post padding in DecomposeAvgPool2dPass when the needed post padding is negative.

Dynamic (SymInt) spatial shapes keep the previous behaviour.

Release notes: arm

Test plan

  • New cases ceil_mode_last_window_dropped / ceil_mode_last_window_dropped_padding in test_max_pool.py and ceil_mode_last_window_dropped / ceil_mode_last_window_dropped_count_include_pad in test_avg_pool2d.py. The TOSA FP/INT/INT a16w8 variants fail on main and pass with this change.
  • Random sweep of ceil_mode max_pool2d/avg_pool2d configurations through TosaPipelineFP and the TOSA reference model: 146/300 fail on main; the remaining 17 failures with this change are all divisor_override + ceil_mode value mismatches, a separate issue (separate PR). Floor mode: 200/200 pass before and after.
  • pytest -c backends/arm/test/pytest.ini backends/arm/test/ops/test_max_pool.py backends/arm/test/ops/test_avg_pool2d.py backends/arm/test/passes (excluding u55/u85/vgf): no new failures.
  • Not run locally: the u55/u85/vgf variants (no Corstone FVP / Vela installed here), so those rely on CI.
  • ufmt check and flake8 clean on the changed files.

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani @nil-is-all

PyTorch drops a last ceil_mode pooling window that would start in the
right padding, so e.g. max_pool2d(kernel=2, stride=4, padding=0,
ceil_mode=True) on a width of 7 gives 2 output columns, the same as
floor mode. The input then has a remainder that no window uses.

SizeAdjustInputPass skipped every ceil_mode pool, so that remainder was
never sliced off and adjust_pooling_pad_if_needed returned a negative
post padding (-1 above). MAX_POOL2D and AVG_POOL2D were emitted with a
negative pad, which the TOSA reference model rejects ("At least one pad
is smaller than zero"). For avg_pool2d with count_include_pad=True,
DecomposeAvgPool2dPass fell back to padding with the full symmetric pad
instead, which made ceil_mode keep the dropped window and produced one
output element too many.

Slice the unused input for ceil_mode pools with static spatial shapes in
every dimension where ceil_mode does not add an output element, and do
not add post padding in DecomposeAvgPool2dPass when the needed post
padding is negative.

Test plan: Lowered 400 ceil_mode max_pool2d/avg_pool2d configurations
(kernel 1-3, stride 1-4, padding 0-1, count_include_pad, divisor_override,
input height 4-7) through TosaPipelineFP: 169 failed before, the
remaining 40 failures are all padding=0 with divisor_override, which is
a separate issue. Added test cases to test_max_pool.py and
test_avg_pool2d.py.

Signed-off-by: Eylon Eliyahu Krause <64223519+EylonKrause@users.noreply.github.com>
@pytorch-bot

pytorch-bot Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24893

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 16 Awaiting Approval

As of commit 4f22175 with merge base 81acbc1 (image):

AWAITING APPROVAL - The following workflows need approval before CI can run:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 11, 2026
@linux-foundation-easycla

linux-foundation-easycla Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: EylonKrause / name: Eylon Eliyahu Krause (b9e74bd)

@github-actions github-actions Bot added ciflow/trunk module: arm Issues related to arm backend labels Nov 11, 2026
@pytorch-bot

pytorch-bot Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

The following ciflow label(s) have been added but CI has not been triggered yet because the workflows are awaiting approval:

  • ciflow/trunk

Once a maintainer approves the workflows (scroll to the bottom of the PR page), the corresponding CI jobs will be triggered automatically. Please ping one of the reviewers if you do not have access to approve and run workflows.

@executorch-triage executorch-triage Bot added the community: contribution PRs coming from community (excluding hardware partners) label Nov 11, 2026
@zingo zingo added the partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm label Nov 11, 2026
@zingo

zingo commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Thanks a lot for this PRs :) I kicked of some extended testing. As this is the first contribution to the project you will need to sign a EasyCLA, I hope that is OK, instructions are in a comment in this issue see #24893 (comment) :)

Return a plain bool from _ceil_mode_adds_output (mypy: the comparison of
two adjust_pooling_pad_if_needed results is typed bool | SymBool), and
rewrap its docstring as docformatter expects.

Signed-off-by: Eylon Eliyahu Krause <64223519+EylonKrause@users.noreply.github.com>
@EylonKrause
EylonKrause force-pushed the fix/pool-ceil-mode-dropped-window branch from 91dc013 to 4f22175 Compare November 13, 2026 13:18
@EylonKrause

Copy link
Copy Markdown
Author

@pytorchbot label "release notes: arm"

@EylonKrause

Copy link
Copy Markdown
Author

Thanks @zingo! I pushed a follow-up commit for the lintrunner findings: _ceil_mode_adds_output now returns a plain bool (mypy saw bool | SymBool), and its docstring is rewrapped for docformatter. The WebGPU jobs failed with curl: (22) download errors, and the unittest failures are also on the merge base, so those look unrelated.

@pytorch-bot pytorch-bot Bot added the release notes: arm Changes to the ARM backend delegate label Nov 13, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. community: contribution PRs coming from community (excluding hardware partners) module: arm Issues related to arm backend partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm release notes: arm Changes to the ARM backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants