Qualcomm AI Engine Direct - Pytest Refactor for Passes

[winskuo-quic]

Summary

Pytest Framework enabling test for passes.

Sample script:
pytest backends/qualcomm/tests/rework/passes/test.py

Test plan

Passing all pytest test and legacy CI test

Author: @haowhsu-quic , @winskuo-quic

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 2 New Failures

As of commit 8093819 with merge base 94c6c88 ():

NEW FAILURES - The following jobs have failed:

• pull / unittest / linux / linux-job (gh)
  RuntimeError: Command docker exec -t 951f1ed119bb7f445dd067e699b0c0255afd57ac09c4e3abbbe57e04a953c2bc /exec failed with exit code 1
• pull / unittest-editable / linux / linux-job (gh)
  RuntimeError: Command docker exec -t 4b82b3ddb7fa5e0d7770ea1f8a0d2bd933db16488a5ef8c3d2c534cf68d5cfd8 /exec failed with exit code 1

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

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[psiddh]

@claude review

[claude]

Claude finished @psiddh's task in 5m 36s —— View job

---

PR Review: Pytest Refactor - Passes

• Gather context (read changed files)
• Review pass changes (_passes/)
• Review new test framework (tests/rework/passes/)
• Post review feedback

Reviewed the pass simplifications and the new pytest-based pass-testing framework. Overall this is a clean, well-structured refactor — the fixture plumbing (enumerate_backends / repack_pass_fixtures / unpack_pass_fixtures) and the Assertions / PassPipeline helpers are nicely factored, and the pass changes mostly remove now-redundant edge-dialect handling as passes move to the pre-edge pipelines. CI is green. A few things worth addressing below.

---

Correctness / robustness

1. decompose_any.py — substring regex can false-match

if re.search(r"any\.(dim|default)", str(node.target)):

re.search matches anywhere in the string, so a target whose stringified name contains any.dim / any.default (e.g. a hypothetical ...many.dim...) would match unintentionally. This substring risk pre-existed with "any.dim" in str(...), but note that decompose_expm1.py in this same PR does the clean thing — matching against an explicit op-overload set:

if node.target in {torch.ops.aten.special_expm1.default, torch.ops.aten.expm1.default}:

Recommend the same set-membership approach here (torch.ops.aten.any.dim, torch.ops.aten.any.default) for consistency and safety. Fix this →

2. decompose_maxpool3d.py — return_indices detection

return_indices = (
    any(user.args[1] == 1 for user in node.users)
    if len(node.meta["val"]) > 1
    else False
)

The new user-traversal approach (detecting whether the indices output is actually consumed) is a real improvement over trusting node.args[6], and the len(node.meta["val"]) > 1 guard correctly excludes the single-output variant. One assumption to flag: every user of a tuple-output node is expected to be a getitem with an index in args[1]. That holds for the standard max_pool3d_with_indices lowering, so it's fine in practice, but a stray non-getitem user would raise IndexError. Worth a defensive user.op == "call_function" / target check or a short comment locking in the getitem assumption.

Also note the changed return → continue: previously the pass bailed on the first unsupported node, now it correctly skips just that node and keeps processing the rest. Good catch — that's a latent bug fix.

Nits

3. Stale comments in DecomposeVar test (src/pattern.py)
This PR removed the create_const_node / const_cache scale-node logic from decompose_var.py (scale is now a plain Python float). The test comments still describe the old behavior:

• L3332: "adds mul + get_attr scale node"
• L3410: "mul×2 (diff² + scale) + get_attr scale const"

The assertions themselves are correct (they only count mul), but the comments now describe a get_attr node that no longer exists. Recommend updating them to avoid confusing future readers.

4. Dead assertion in PassPipeline.lower_edge_ep (passes_helper.py:253)

edge_passes = PassPipeline._slice_to_target(...)
assert edge_passes is not None, f"{target_pass.__name__} not found in edge passes"

_slice_to_target already asserts internally and never returns None, so this follow-up assert is unreachable. Minor cleanup.

5. default_property() in conftest.py
Defining the Property dataclass inside the function and returning a fresh instance on every call is a slightly unusual pattern. A module-level dataclass would be cleaner and cheaper, but this is cosmetic.

Questions

• The removal of DecomposeCDist, DecomposeFill, and DecomposeVar from get_default_pass_activations() and the dependency table (qnn_pass_manager.py) — these now run only in the annotation/export pipelines rather than the to-edge pipeline. Can you confirm this is intentional and that no downstream path still relied on them firing during to-edge lowering? CI green is reassuring, but a one-line note in the PR description on why these moved would help reviewers.

Nice work overall — the test framework is a solid foundation and the per-pass pattern coverage (backend enumeration, quantized vs fp16, subtests for op variants) is thorough.
· branch dev1/winskuo/test_framework_refactor_passes

[winskuo-quic]

Hi @psiddh,
I have made corresponding changes based on Claude review in the newest commit.
Please let me know if you would like to know more details about this PR.
Thanks

[psiddh]

@claude can you review the latest changes again

[claude]

Claude finished @psiddh's task in 54s —— View job

---

Follow-up Review: Latest Changes

• Identify commits made after the previous review
• Verify each review item was addressed
• Post follow-up review

Reviewed the two new commits (82151e5 Code Review, 8093819 minor). All actionable points from my previous review have been addressed cleanly:

#	Previous feedback	Status
1	decompose_any.py substring re.search false-match risk	✅ Now uses explicit op-overload set membership (exir_ops.edge.aten.any.{dim,dims,default}), re/import re removed
2	decompose_maxpool3d.py return_indices could IndexError on a non-getitem user	✅ Guarded with user.op == "call_function" and user.target is operator.getitem and user.args[1] == 1
3	Stale get_attr scale comments in DecomposeVar test	✅ Updated to "mul by scalar" / "scale by scalar"
4	Dead unreachable assertion in PassPipeline.lower_edge_ep	✅ Removed
5	default_property() defining dataclass inside the fn	✅ Property hoisted to module level; default_property() just returns Property()

Notes on the new changes

• decompose_any.py — the 8093819 commit adds exir_ops.edge.aten.any.dims to the set. Good catch: any.dims (multi-dim reduction) is a distinct overload the old re.search(r"any\.(dim|default)") would have missed entirely, since "any.dims" doesn't contain "any.dim" as a \.-anchored match... actually it would have substring-matched any.dim inside any.dims, so the regex was both over- and under-specified. The explicit set is strictly more correct. The Any module already handles a list dim via tuple(dim), so this overload is handled correctly downstream. 👍

• decompose_maxpool3d.py — the operator.getitem target check is exactly the defensive fix suggested. Since this pass runs on the edge graph where the tuple output is unpacked via getitem, this is the right predicate.

• conftest.py — module-level @dataclass Property is cleaner and avoids re-defining the class per call.

Remaining open item (non-blocking)

The one question from my prior review is still worth a one-line answer for the record: the removal of DecomposeCDist / DecomposeFill / DecomposeVar from get_default_pass_activations() and the qnn_pass_manager.py dependency table — confirming these intentionally now run only in the annotation/export pipelines (not to-edge) and that no downstream path relied on them firing during to-edge lowering. Not a blocker (CI green), just useful context in the PR description.

LGTM on the review-feedback changes — they're all correct and consistent with the surrounding style. Nice work.
· branch dev1/winskuo/test_framework_refactor_passes