Fix XNNPACK rejecting avg_pool2d with default arguments - #21862
Fix XNNPACK rejecting avg_pool2d with default arguments#21862slipstr34m wants to merge 1 commit into
Conversation
AvgPoolingConfig rejected every node with count_include_pad=True, but that is the aten default, so a plain nn.AvgPool2d(k) never delegated. The flag only affects the averaging divisor when padded elements exist; with the default zero padding there is nothing to include, so the rejection excluded nodes XNNPACK handles correctly and its why-log blamed zero-padding the node does not have. The guard now rejects count_include_pad=True only when padding is non-zero, which is the case the backend genuinely does not implement.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21862
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit ddb9251 with merge base ed65b12 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot label "release notes: none" |
This PR needs a
|
Problem
AvgPoolingConfig.check_constraintsrejects every node withcount_include_pad=True:But
count_include_pad=Trueis the aten default, so a plainnn.AvgPool2d(2)— the most common way to write the layer — never delegates to XNNPACK. The delegate splits around it and the node falls back to the portable op, and theWhyNoPartitionlog tells the user their model has zero-padding it does not have:The flag only has semantics when padded elements exist: it controls whether zeros from padding count toward the averaging divisor. With the default
padding=0there is nothing to include or exclude, so the rejection excludes nodes XNNPACK handles correctly. Verified numerically: withpadding=0, eager outputs forcount_include_pad=TrueandFalseare bit-identical, and the delegated result matches eager within ordinary float tolerance; withpadding=1they genuinely differ, which is the case the backend does not implement.Fix
Reject
count_include_pad=Trueonly when the node has non-zero padding, reading padding from the existingnormalize_pool2d_argscall:A no-op for every node the config currently accepts (those all have
count_include_pad=False), and every currently-rejected padded node is still rejected, so the only behavior change is crash-to-delegate for the default-argument case. The visitor needs no change: it serializes the padding values themselves and never readscount_include_pad, and XNNPACK's divisor convention is irrelevant when no padded elements exist.Effect
Same model,
nn.AvgPool2d(2)with all-default arguments, throughto_edge_transform_and_lowerwith the XNNPACK partitioner:Testing
New
test_fp32_avgpool2d_default_argslowersnn.AvgPool2d(2)end to end through serialize and compares outputs against eager. Verified failing-first: with the tests kept and only the source reverted, exactly that test fails and the other seven pass, includingtest_fp32_avgpool2d_count_include_pad_unsupported(padding=(1,1)), which still rejects under the fix.backends/xnnpack/test/ops/test_avgpool2d.py: 7 -> 8 passed. Full backends/xnnpack/test/ops/ suite: 229 passed on main, 230 passed with this change, no regressions. The suite runs exclude test_prelu.py, whose test_fp32_prelu_constant_weight_empty_decompositions_file_load segfaults identically on unmodified main in my environment (prebuilt runtime, macOS arm64), so it cannot observe this change either way. lintrunner reports no issues on both changed files.
cc @GregoryComer @digantdesai @cbilgin @JakeStevens