Repository navigation
Mark the 3-D convolution cases that fail inside a backend as expected failures - #24459
shoumikhin wants to merge 7 commits into
Conversation
… failures Running 5-D convolutions in the backend test suite shows some cases that fail inside the backend that takes them, not in the portable kernel: - QNN quantized flows: depthwise 3-D convolutions and the padded input of padding_modes fail to lower, and the delegated transposed groups, depthwise and padding cases do not match. With 16-bit activations and 4-bit weights, QNN fails to build a context binary for every 3-D case. - Ethos-U85: the delegated stride and groups cases do not match. - Ethos-U55: the runner is built without the quantized_decomposed::dequantize_per_channel.out kernel that padding_modes leaves outside the delegate. - Core ML: the reflect padding case fails to compile, and lowering a transposed convolution with output_padding raises a TypeError. - WebGPU: replicate padding fails to build the graph. Each flow lists those tests as strict expected failures, so the report keeps recording how they fail, and a fix in the backend shows up as an unexpected pass.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24459
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 Cancelled Job, 1 Unrelated FailureAs of commit a2aac04 with merge base 76efad5 ( CANCELLED JOB - The following job was cancelled. Please retry:
BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@claude /pr-review |
meta-cla
Bot
added
the
CLA Signed
|
Claude finished @shoumikhin's task in 15m 40s —— View job Requesting changes: the Ethos-U55 entry xfails a missing kernel in the Arm test runner build, which Detailed reviewPR Review: #24459SummaryThis PR adds strict per-flow Infrastructure
Testing
Code Quality
RecommendationRequest Changes Replace the Ethos-U55 entry with the runner-build fix that the Arm setup already uses for this kind of failure. Before merging, also confirm that the QNN and U85 mismatch cases fail by a wide margin. A strict xfail on a mismatch near the threshold will turn the nightly red at random. |
…per-channel dequantize test_conv3d_padding_modes ran four padding modes in one test, and the first failing mode stopped the rest. Each mode is now its own test, so a flow lists only the mode that fails there, and the other modes keep running. The Ethos-U55 reflect padding case failed only because the Corstone test runner was built without quantized_decomposed::dequantize_per_channel.out. The runner now includes that kernel, the same way the Arm test setup already adds other kernels that flow-suite cases leave outside the delegate, so the case runs instead of being listed as an expected failure. The QNN lists are split into the cases that crash QNN partitioning, which fail in every quantized flow, and the transposed cases that do not match with 8-bit weights.
|
Thanks. Changes in the new commit:
On the flake risk: the seed does change per run, but the mismatches are far outside the tolerance. The largest differences were 2.0 to 3.8 against |
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 20m 32s —— View job Needs discussion: the step summary ignores pytest outcomes, so the new lists change nothing there and a fixed case would show up as a plain Pass, and the QNN lists were recorded against two pre-existing flow misconfigurations ( Detailed reviewPR Review: #24459SummaryThis PR adds strict per-flow Code Quality
Infrastructure
RecommendationNeeds Discussion Before merging, decide where the summary gap is closed. I recommend making |
The step summary counted each subtest by the runner's own result and never read pytest's outcome. So a test in a flow's xfail_patterns still counted as a failure and was listed under Failed Tests, and one that started passing showed up as an ordinary pass, although pytest reports it as a strict unexpected pass. The summary now counts failures of listed tests as expected failures, keeps them out of Failed Tests, and lists strict unexpected passes in their own section, so a stale entry is easy to spot.
test_conv3d_padding already runs a zeros-padded 3-D convolution on the same input, so a separate zeros test adds nothing. The qnn_16a4w_block flow still quantizes as 8a8w; the comment now says so as a gap, so whoever fixes the dtype knows to recheck its list.
The nightly run of the split padding tests shows that every quantized QNN flow fails to lower the circular-padded 3-D convolution: the QNN graph's inputs and outputs do not match the delegated program. Ethos-U55, Ethos-U85 and WebGPU pass it, and Ethos-U55 now also passes reflect padding with the per-channel dequantize kernel in its runner.
|
Thanks, addressed in the new commits:
You are right that One correction to my earlier reply: most of the mismatch cases ran on the same inputs in both nightlies, so that part was not extra evidence. The size of the errors is. |
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 21m 26s —— View job Approve: no blockers remain; the four minor points are that the step summary shows expected failures only as a count, the new summary logic has no test, three listed tests still hide their second configuration, and the Detailed reviewPR Review: #24459SummaryThis PR marks the 3-D convolution cases that fail inside the QNN quantized, Ethos-U85, Core ML and WebGPU backends as strict expected failures. It also splits the padding-modes test, adds per-channel dequantize to the Ethos-U55 runner, and makes the step summary read pytest's outcome. No blockers. Four minor points remain: the summary keeps only a count for expected failures, the new summary logic is untested, three listed tests still run two configurations each, and one QNN comment names the wrong cause. Code Quality
Infrastructure
Testing
RecommendationApprove None of these blocks merging. The step summary point matters most. Right now a listed case that starts failing in a different way shows up only in the JSON report. |
The summary counted expected failures but no longer showed their details, so a test in xfail_patterns that started failing in a different way, for example a lowering failure instead of an output mismatch, was visible only in the JSON report. Expected failures now get their own table, like Failed Tests. New unit tests check the counts, which records go to the expected failure and failed test lists, and the strict unexpected pass detection, both for a serial run and for a pytest-xdist run, where the report text starts with the worker's name.
The depthwise 3-D convolution case fails in QNN partitioning in this flow too, before any context binary is built, so the comment no longer says that every case fails while generating the context binary.
|
Thanks. Addressed in the new commits:
On the tests with two configurations: in the QNN 8-bit-weight flows, |
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 21m 37s —— View job Approve: no blockers. Four minor points remain: a listed case that times out shows up in no table of the step summary, unexpected passes are found by parsing pytest's message text, nothing tests Detailed reviewPR Review: #24459SummaryThis PR marks the 3-D convolution cases that fail inside the QNN quantized, Ethos-U85, Core ML and WebGPU backends as strict expected failures. It also splits the 3-D padding-mode test, adds per-channel dequantize to the Ethos-U55 runner, and makes the step summary list expected failures and strict unexpected passes. No blockers. Four minor points remain, and three of them are in the new summary logic. Infrastructure
Testing
Code Quality
RecommendationApprove None of these blocks merging. The timeout gap matters most. The Expected Failures table is meant to keep a listed case visible when it starts failing in a new way, and a hang is a case it misses. |
|
Thanks. I updated the description's 16a4w sentence to match the code comment. The other three points are fair, and I am leaving them out of this pull request on purpose:
|
The backend test suite now runs 3-D convolutions, since the portable kernel supports them (#24385). In the nightly flows, a few of those cases fail inside the backend that takes them, not in the portable kernel. This change marks exactly those cases as expected failures, per flow, with the reason next to each list. It also fixes one case that failed only because of the test runner build.
What fails, by flow:
groups,depthwiseandpaddingcases do not match the reference. With 16-bit activations and 4-bit weights, every 3-D case fails, most of them when QNN builds the context binary.strideandgroupscases do not match.output_paddingraises aTypeError.clampdoes not take an integer type).Three smaller changes make the lists exact and visible:
test_conv3d_padding_modesran four padding modes in one test, and the first failing mode stopped the rest. Reflect, replicate and circular are now separate tests (test_conv3d_paddingalready covers zeros), so a flow lists only the mode that fails there.quantized_decomposed::dequantize_per_channel.out. The runner now includes it, the same way the Arm test setup adds other kernels that suite cases leave outside the delegate.How the failures were sorted: every 3-D convolution failure in the nightly run of #24385 was read from the job logs and traced to its backend. None is in the portable kernel. The CPU, XNNPACK, Arm TOSA and VGF, Vulkan, NXP and OpenVINO flows pass every case. The QNN floating point flow stops on an unrelated error before it reaches these cases.
The output mismatches are far outside the tolerance (a largest difference of 2 to 3.8 against
atol=0.1), and the same cases failed in the nightly run on this pull request. The lists use the same strict expected-failure mechanism as the Cortex-M flow. The cases still run and still show up in the report. When a backend fixes one, the summary lists it under Unexpected Passes, and the entry can be removed.The QNN, Arm and Core ML flow helpers gain an optional
xfail_patternsargument. No other behavior changes.Test plan: checked against the operator test reports of the nightly run of #24385 and of the nightly run on this pull request, after the split. Every pattern matches only cases that failed in that flow, and every 3-D convolution failure is matched. On this pull request, Ethos-U55 passes all 3-D cases, including reflect padding, and every listed case shows as an expected failure. The Core ML flow did not run there (no macOS runner picked it up), so its replicate and circular padding results are still unknown. The summary change was checked on the reports of that run: for example, it finds two strict unexpected passes in the NXP flow (
test_lstm_seq_lengthsandtest_lstm_num_layers), which the old summary showed as ordinary passes.