Qualcomm AI Engine Direct - Test Framework Refactor #2

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.

[haowhsu-quic]

Summary

• fill the test gap for newly added op
• utils test

Test plan

pytest backends/qualcomm/tests/rework/utils/test.py

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Cancelled Job

As of commit acb65ce with merge base 4af91c3 ():

NEW FAILURE - The following job has failed:

• pull / test-binary-size-linux-gcc / linux-job (gh)
  RuntimeError: Command docker exec -t c128ac777eefe4bcf151320c13e75bcba1c7b3045f2923552f1dbd06b0edb9d9 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-nxp-neutron / linux-job (gh)
  ##[error]The operation was canceled.

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 2m 15s —— View job

---

Claude PR Review

Review order suggestion:src/op.py (new op modules) → htp/op/v68/test.py (test hookups) → src/utils.py (utils test bodies) → utils/conftest.py + conftest.py (fixtures).

Overall this is a clean, well-structured expansion of the rework test framework. The new op test modules follow the existing unpack_fixtures / export_and_verify conventions consistently, and the utils.py test bodies are a solid addition covering previously-untested utility paths (dump_context_from_pte, draw_graph, mixed precision, skip-node, QAT, etc.). Comments below are mostly minor.

---

1. DrawGraph golden comparison is order-insensitive (src/utils.py:352)

assertsorted(__class__._golden.split()) ==sorted(result.split()), ...

Comparing the sorted multiset of whitespace-split tokens makes the assertion insensitive to structure and edge ordering. A malformed .dot where edges are connected differently (e.g. A -> B vs B -> A) but the token multiset is identical would still pass, since ->, "A", "B" all appear the same number of times. The comment "does not match the golden file" implies an exact match. Consider comparing line-sets (sorted(golden.splitlines()) after stripping) so edge/node lines stay atomic, or normalizing whitespace per-line instead of globally. Not blocking, but it weakens the test's guarantee.

2. self.modules shadows nn.Module.modules() (src/utils.py:78, _CompositeDelegateModule)

Assigning self.modules = [...] overrides the built-in nn.Module.modules() method. This mirrors the pre-existing CompositeDelegateModule in backends/qualcomm/tests/models.py:521, so it's consistent with the repo — flagging only so it's a conscious choice rather than a copy-forward. A name like self.submodels would avoid the shadowing.

3. Alphabetical ordering broken for Var / test_var

Both op.py (Var appended after Where) and v68/test.py (test_var after test_where) place Var/var out of the otherwise-alphabetical order the rest of the file maintains. Minor, but the file is clearly organized alphabetically elsewhere — placing them between SwapAxes/Tan and test_swapaxes/test_tan would keep it consistent.

4. default_property() returns a fresh dataclass instance each call (conftest.py:67)

defdefault_property():
@dataclassclassProperty:
soc_model: str="SM8750"returnProperty()

Defining the dataclass inside the function re-creates the class on every call and returns a new instance. Since it only carries a constant default, a module-level singleton (or plain constant) would be simpler and avoids re-declaring the class per invocation. Works as-is, just heavier than needed for "extend this for backend agnostic tests."

5. Minor style nits

• RewritePreparedObserver.test and QAT.test use local import math / from torchao... inside the function body. A couple of these are reasonable to defer heavy imports, but import math at line 403 could live at module top.
• MultiContextsComposite.test (utils.py:397) and MultiContextsComposite builds the composite but only asserts "should complete without error" via to_executorch(). Consider a lightweight output/partition-count check to make the assertion explicit, matching the more thorough checks in SkipNodePartitioner.

---

CI status

The reported test-binary-size-linux-gcc failure and the cancelled unittest-nxp-neutron job appear unrelated to these Qualcomm test-only changes — worth a rerun to confirm they're not flaky/infra.

Nice coverage additions overall — the op set (addmm, alias, div rounding modes, fill, scatter, select_scatter, tan, var) and the utils suite meaningfully close test gaps. None of the above is blocking.