Skip to content

Conversation

daxfohl
Copy link
Collaborator

@daxfohl daxfohl commented Mar 3, 2025

When decomposing CCCCCX and such large multi-controlled gates, intermediate MatrixGates get created (notably not controlled MatrixGates, which would be problematic; just top-level MatrixGates as part of the decomposition). These then get further decomposed to a different set of gates.

On some systems, due to rounding error, they get decomposed to a different set of gates that usual, which are equivalent but have different global phase than the standard decomposition. Again, these are top-level MatrixGates, not controlled ones, so its "okay" that their decompositions have different global phase. However, the test tested for np.allclose, and this test would fail on such systems. This PR changes the test to cirq.equal_up_to_global_phase, which should fix the tests on those systems.

xref #7071 (comment)

Also adding back some test parameters I'd removed earlier because they were failing on Windows, and I'd assumed it was due to general rounding error; it didn't occur to me that it could be a phase difference. Let's see if the change allows them to pass.

@daxfohl daxfohl requested review from vtomole and a team as code owners March 3, 2025 07:16
@daxfohl daxfohl requested a review from 95-martin-orion March 3, 2025 07:16
@CirqBot CirqBot added the size: S 10< lines changed <50 label Mar 3, 2025
@daxfohl daxfohl marked this pull request as draft March 3, 2025 07:18
Copy link

codecov bot commented Mar 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.15%. Comparing base (2ad4136) to head (741e4fa).
Report is 21 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7112      +/-   ##
==========================================
- Coverage   98.15%   98.15%   -0.01%     
==========================================
  Files        1089     1089              
  Lines       95251    95250       -1     
==========================================
- Hits        93497    93495       -2     
- Misses       1754     1755       +1     

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@daxfohl daxfohl marked this pull request as ready for review March 3, 2025 07:28
@daxfohl daxfohl enabled auto-merge March 3, 2025 07:28
@daxfohl
Copy link
Collaborator Author

daxfohl commented Mar 3, 2025

cc @pavoljuhas

@daxfohl
Copy link
Collaborator Author

daxfohl commented Mar 12, 2025

Replacing with 7118

@daxfohl daxfohl closed this Mar 12, 2025
auto-merge was automatically disabled March 12, 2025 15:02

Pull request was closed

@daxfohl daxfohl deleted the fix-controlled-decompose-test branch April 5, 2025 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: S 10< lines changed <50
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants