Repository navigation
Commit 9ffbe36
authored
Fix XNNPACK rejecting avg_pool2d with default arguments (pytorch#21862)
## Problem
`AvgPoolingConfig.check_constraints` rejects every node with
`count_include_pad=True`:
```python
if count_include_pad:
why(
node,
reason="zero-padding in the averaging calculation is not supported",
)
return False
```
But `count_include_pad=True` is the aten default, so a plain
`nn.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 the `WhyNoPartition` log tells the user
their model has zero-padding it does not have:
```
WhyNoPartition: Node aten_avg_pool2d_default was not partitioned because zero-padding in the averaging calculation is not supported.
```
The flag only has semantics when padded elements exist: it controls
whether zeros from padding count toward the averaging divisor. With the
default `padding=0` there is nothing to include or exclude, so the
rejection excludes nodes XNNPACK handles correctly. Verified
numerically: with `padding=0`, eager outputs for
`count_include_pad=True` and `False` are bit-identical, and the
delegated result matches eager within ordinary float tolerance; with
`padding=1` they genuinely differ, which is the case the backend does
not implement.
## Fix
Reject `count_include_pad=True` only when the node has non-zero padding,
reading padding from the existing `normalize_pool2d_args` call:
```python
kernel_size, _, padding, _ = normalize_pool2d_args(node, has_dilation=False)
...
if count_include_pad and any(p != 0 for p in padding):
```
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 reads `count_include_pad`, and
XNNPACK's divisor convention is irrelevant when no padded elements
exist.
## Effect
Same model, `nn.AvgPool2d(2)` with all-default arguments, through
`to_edge_transform_and_lower` with the XNNPACK partitioner:
```
before: delegates=0 remaining aten.avg_pool2d.default in graph, WhyNoPartition blames zero-padding
after: delegates=1 no avg_pool2d left in graph, output matches eager
guard: padding=(1,1) with count_include_pad=True still not delegated
```
## Testing
New `test_fp32_avgpool2d_default_args` lowers `nn.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, including
`test_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 @JakeStevens1 parent 3dfa644 commit 9ffbe36
2 files changed
Lines changed: 32 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
168 | 168 | | |
169 | 169 | | |
170 | 170 | | |
171 | | - | |
| 171 | + | |
| 172 | + | |
172 | 173 | | |
173 | 174 | | |
174 | 175 | | |
| |||
184 | 185 | | |
185 | 186 | | |
186 | 187 | | |
187 | | - | |
| 188 | + | |
188 | 189 | | |
189 | 190 | | |
190 | 191 | | |
| |||
194 | 195 | | |
195 | 196 | | |
196 | 197 | | |
197 | | - | |
| 198 | + | |
198 | 199 | | |
199 | 200 | | |
200 | 201 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
70 | | - | |
| 70 | + | |
| 71 | + | |
71 | 72 | | |
72 | 73 | | |
73 | 74 | | |
| |||
78 | 79 | | |
79 | 80 | | |
80 | 81 | | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
81 | 108 | | |
82 | 109 | | |
83 | 110 | | |
| |||
0 commit comments