Skip to content

Speed up gridsynth integration tests - #3400

Open
Qottmann wants to merge 5 commits into
mainfrom
qottmann/faster-gridsynth-tests
Open

Qottmann wants to merge 5 commits into
mainfrom
qottmann/faster-gridsynth-tests

Conversation

@Qottmann

@Qottmann Qottmann commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

AI-assisted PR (Claude, in Cursor). The description below was drafted by the assistant and is pending author review.

Context: frontend/test/pytest/test_gridsynth.py takes about 2.5 minutes. Almost all of it is compilation: most of its 333 tests compile a fresh qjit program (~0.35 s) to run it on a single angle, although the angle is a runtime argument.

Description of the Change:

  • test_PhaseShift_gridsynth, test_adjoint_gridsynth, test_adjoint_region_gridsynth, test_gridsynth_ppr_basis and test_mixed_gridsynth_samples compile once and loop over the same angles as before, instead of parametrizing over them. The epsilons and other parameters are unchanged.
  • test_mixed_gridsynth_samples and test_mixed_fallback_gridsynth_samples no longer parametrize over PhaseShift. DecomposeCustomOpPattern lowers PhaseShift to the same decomposition call as RZ for every method, followed by a GlobalPhase, which the fidelity checks of these tests cannot see. test_PhaseShift_gridsynth still covers PhaseShift with a phase-sensitive state comparison.

Benefits: test_gridsynth.py goes from 333 tests in ~143 s to 65 tests in ~38 s (local, -n0). The remaining time is mostly the statistical tests (test_mixed_fallback_gridsynth_samples, test_gridsynth_specs_matches_runtime), which were left as they are. The C++ RSDecomp tests (~4.5 s) and the MLIR and frontend lit tests for gridsynth (~5 s) were not changed.

Possible Drawbacks: None expected. A failing looped assert still reports the angle: pytest shows it in the asserted call, e.g. circuit(0.3), and the two asserts that compare derived values (test_gridsynth_ppr_basis, test_mixed_gridsynth_samples) name it in their message.

Related GitHub Issues: None

Qottmann and others added 2 commits October 26, 2026 17:39
Compile each gridsynth circuit once and run it for all angles, instead of
parametrizing over the angle, which is a runtime argument. Drop PhaseShift
from the mixed sampling tests: the gridsynth pattern lowers PhaseShift to the
same decomposition call as RZ for all methods plus a GlobalPhase, which the
fidelity checks of these tests cannot see. test_PhaseShift_gridsynth still
covers PhaseShift with a phase-sensitive state comparison.

test_gridsynth.py goes from 333 tests in ~143 s to 65 tests in ~38 s.

Assisted-by: Claude (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
Assisted-by: Claude (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
@Qottmann
Qottmann marked this pull request as ready for review October 26, 2026 16:44
Assisted-by: Claude (Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
@Qottmann
Qottmann force-pushed the qottmann/faster-gridsynth-tests branch from c47a28a to ecfda72 Compare October 27, 2026 11:05
@Qottmann

Copy link
Copy Markdown
Contributor Author

Jukebox review (Opus 5.5)

Summary

This PR only changes tests. The angle is already a runtime argument (x: float), so recompiling for every angle was wasted time. The PR now compiles each qjit program once per remaining parameter combination and loops over the same angles inside the test. The epsilons and tolerances are unchanged. It also drops the PhaseShift parametrization from the two mixed-method sampling tests. Reported result: 333 tests in about 143 s become 65 tests in about 38 s.

Checks

Dropping PhaseShift from the mixed tests is safe. In mlir/lib/Quantum/Transforms/GridsynthPatterns.cpp, DecomposeCustomOpPattern sends PhaseShift to the same __catalyst_decompose_RZ* function as RZ, for every method. The only extra step is a GlobalPhase(-phi/2). Both mixed tests measure fidelity in a way that ignores global phase (density matrix in one, absolute overlap in the other), so the PhaseShift cases were duplicates.
PhaseShift is still covered where phase matters. test_PhaseShift_gridsynth and test_adjoint_gridsynth compare full states. test_adjoint_region_gridsynth still covers PhaseShift with ppr_basis.
The new tests are slightly stronger. One compiled binary now runs several angles in a row, so they would also catch state leaking between calls in the runtime decomposition path.
Same angles as before. The angle lists match the old parametrizations exactly; test_PhaseShift_gridsynth builds the same 17 values in a different order.
CI passes: 26 checks pass, including all three Frontend Tests jobs; the 12 skipped are wheel-build jobs. I did not run the suite locally, so the timing figures are the author's.

Verdict: Approve. The change is small and correct, the coverage reduction is justified by the compiler code, and it cuts the file's runtime by about 4x.

Qottmann and others added 2 commits October 27, 2026 14:47
@Qottmann
Qottmann requested review from albi3ro and dime10 October 27, 2026 13:52
@codecov

codecov Bot commented Oct 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.21%. Comparing base (5df1e64) to head (8091b80).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3400   +/-   ##
=======================================
  Coverage   96.21%   96.21%           
=======================================
  Files         176      176           
  Lines       21583    21583           
  Branches     2160     2160           
=======================================
  Hits        20766    20766           
  Misses        631      631           
  Partials      186      186           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@albi3ro albi3ro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

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