Skip to content

Raise inoperative error guards in ExportSession, XNNPACK node_visitor, and Samsung conv1d - #22373

Open
AxelNoun wants to merge 1 commit into
pytorch:mainfrom
AxelNoun:fix/unraised-error-checks
Open

Raise inoperative error guards in ExportSession, XNNPACK node_visitor, and Samsung conv1d#22373
AxelNoun wants to merge 1 commit into
pytorch:mainfrom
AxelNoun:fix/unraised-error-checks

Conversation

@AxelNoun

Copy link
Copy Markdown
Contributor

Summary

Four error-guard sites construct a failure that cannot actually fire. This PR inserts the missing raise (or replaces an always-true assert with the file's existing check_or_raise) so the documented failure mode is the one callers see.

File What happens today Reachability
export/export.py ExportSession.print_delegation_info, empty lowering-stage set RuntimeError(...) is built and discarded; lowering_stage[0] then raises IndexError: list index out of range Reachable. ExportSession is in executorch.export.__all__ and is @experimental. Calling print_delegation_info() on a session whose pipeline has no lowering stage hits this path.
export/export.py print_delegation_info, lowering stage listed but not run RuntimeError(...) is built and discarded; None.get_context(...) then raises AttributeError: 'NoneType' object has no attribute 'get_context' Reachable. A pipeline that lists TO_EDGE_TRANSFORM_AND_LOWER (or TO_BACKEND) but has not run that stage hits this path.
backends/xnnpack/operators/node_visitor.py per-channel axis else assert f"Unsupported weight per channel quantization axis ..." — the f-string is always truthy, so the assertion never fires. An assert would also disappear under python -O. Not reachable on a normal export. The only call that passes swap_in_out_for_weights=True is backends/xnnpack/operators/op_conv2d.py:102, and that visitor already check_or_raises the axis to 0 (depthwise) or 1 (transpose) before define_tensor. This change restores a guard that can actually raise if that else is ever entered; it does not fix a mis-serialized .pte on the current conv2d path.
backends/samsung/_passes/conv1d_to_conv2d.py update_kernel else RuntimeError("Weight of 1d conv should be constant tensor or Parameter obj") is built and discarded; weight_node.meta["val"] is still unsqueezed. Not reachable on a normal Parameter / lifted-constant conv1d export. Those two cases are handled by the preceding branches. The else is the leftover for a weight that is none of those. This change only makes that documented RuntimeError actually raise if the branch is entered.

print_delegation_info also corrects the message "atleast" → "at least".

Test plan

Tests are included only for the two reachable sites (F3a). There is intentionally no test for the XNNPACK else (F2) and none for the Samsung else (F3b): both are unreachable on a normal export, and a test that injects that state would exercise a situation the production callers do not produce.

export/tests/test_print_delegation_info.py covers the two ExportSession guards (export/tests is already in pytest.ini testpaths):

pytest export/tests/test_print_delegation_info.py

On 6755ea388ce6d2249d26f568c286a153ccba4883:

  • unpatched: IndexError: list index out of range and AttributeError: 'NoneType' object has no attribute 'get_context'
  • patched: both tests pass with RuntimeError and the documented messages

F2: op_conv2d.py already rejects axis ∉ {0, 1} before define_tensor. No AST test and no injected-axis test is in this PR.

F3b: a normal Parameter / lifted-constant conv1d does not enter the else. No monkeypatch test is in this PR. The change only restores the RuntimeError the source already spelled out, if that branch is ever entered.

A separate, two-line cleanup (duplicate dataclass fields in examples/models/llama/model_args.py and exir/serde/schema.py) is available as differe-f4.patch if a follow-up PR is wanted; it is not part of this change.

@AxelNoun
AxelNoun requested a review from digantdesai as a code owner August 31, 2026 22:32
@pytorch-bot

pytorch-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

🔗 Helpful Links

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

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

⚠️ 14 Awaiting Approval

As of commit 95b4eff with merge base c27baa8 (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 Aug 31, 2026
@AxelNoun

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "release notes: none"

@pytorchbot label "release notes: none"

@pytorch-bot pytorch-bot Bot added the release notes: none Do not include this in the release notes label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants