Skip to content

test(quantization): expand externalized composite ops coverage to rope and gather_mm - #131

Merged
pkmandke merged 4 commits into
apple:mainfrom
rohith500:feat/test-externalize-composite-ops
Oct 21, 2026
Merged

pkmandke merged 4 commits into
apple:mainfrom
rohith500:feat/test-externalize-composite-ops

Conversation

@rohith500

Copy link
Copy Markdown
Contributor

Expands test coverage in tests/quantization/test_composite_op_externalize.py to cover RoPE and GatherMM from coreai_torch.composite_ops, which were previously only exercised with SDPA and RMSNorm.

  • Added CompositeRoPEModel and CompositeGatherMMModel along with their ExternalizeSpec helpers in tests/models/composite.py.
  • Parameterized test_composite_op_survives_prepare_and_finalize across SDPA, RoPE, and GatherMM for both weight-only and w8a8 quantization.
  • Parameterized test_composite_boundary_quantized and test_composite_boundary_input_index_selects_those_args to ensure multi-tensor inputs handle selective input port boundary quantization correctly.
  • Extracted shared tensor sample helpers (_single_tensor_sample, _rope_sample, _gather_mm_sample) to keep test fixtures clean.

Tested with pytest tests/quantization/test_composite_op_externalize.py (20 passed) and make check.

…e and gather_mm

- Add rope_externalize_spec and gathermm_externalize_spec with CompositeRoPEModel and CompositeGatherMMModel
- Parameterize test_composite_op_survives_prepare_and_finalize across SDPA, RoPE, and GatherMM for w8 and w8a8 quantization modes
- Parameterize test_composite_boundary_quantized and test_composite_boundary_input_index_selects_those_args for multi-input composite ops
- Extract reusable sample tensor generators to keep test tables clean and maintainable
@aseemw
aseemw requested a review from pkmandke October 5, 2026 18:03

@pkmandke pkmandke left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rohith500 thanks for helping expand test coverage for externalized composite ops!

My main comment is that we can restrict the structural tests (such as test_composite_boundary_quantized and test_composite_boundary_input_index_selects_those_args) to unique cases- and RoPE seems to cover the exact same structural guarantees as SDPA.

Having said that, I like the idea of having extensive coverage in the E2E test_composite_op_survives_prepare_and_finalize test which you have added already.

@rohith500

Copy link
Copy Markdown
Contributor Author

Thanks for the review @pkmandke! Updated. Removed CompositeRoPEModel from the structural boundary tests (test_composite_boundary_quantized and test_composite_boundary_input_index_selects_those_args) since SDPA already exercises the 3-input arity, while retaining RoPE in the E2E test_composite_op_survives_prepare_and_finalize. All tests passing cleanly.

@pkmandke
pkmandke enabled auto-merge (squash) October 21, 2026 17:15
@pkmandke
pkmandke merged commit 712f62a into apple:main Oct 21, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants