Fix decompose for controlled CZ gates with phase shift #7071
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7071 +/- ##
=======================================
Coverage 98.18% 98.18%
=======================================
Files 1089 1089
Lines 95237 95267 +30
=======================================
+ Hits 93508 93538 +30
Misses 1729 1729 ☔ View full report in Codecov by Sentry. |
daxfohl
commented
Feb 18, 2025
dstrain115
approved these changes
Feb 26, 2025
|
@daxfohl - starting with this PR I am getting the following pytest failure on my Debian-linux desktop - check/pytest -n0 -vv \
"cirq-core/cirq/ops/controlled_gate_test.py::test_controlled_gate_is_consistent[gate13-True]"OutputThe failing test Cirq/cirq-core/cirq/ops/controlled_gate_test.py Lines 403 to 410 in 61548e0 but their phase is consistently different by 0.75pi - (phases in the decomposed-circuit unitary are 0.75pi or 0.25pi, phases in the gate unitary are 0 or -0.5pi respectively). Any idea where this might be coming from? The strange thing is the test passes on a cloud-hosted machine with the same Python version and same libraries in virtual environment. cc @kmlau |
|
@pavoljuhas IDK offhand. Does it decompose to the same thing on both machines? On my machine (ubuntu WSL) it decomposes to |
|
@daxfohl - I used this patch to save the decomposed circuit in test 13 just before the failing assertion diff --git a/cirq-core/cirq/ops/controlled_gate_test.py b/cirq-core/cirq/ops/controlled_gate_test.py
index 1a9f44e2..018cf04b 100644
--- a/cirq-core/cirq/ops/controlled_gate_test.py
+++ b/cirq-core/cirq/ops/controlled_gate_test.py
@@ -424,3 +424,3 @@ def test_unitary():
(C0C_xorH, False),
- ],
+ ][13:14],
)
@@ -481,2 +481,3 @@ def _test_controlled_gate_is_consistent(
circuit = cirq.Circuit(first_op, *decomposed)
+ cirq.to_json(circuit, "decomposed_circuit.json")
np.testing.assert_allclose(cirq.unitary(cgate), cirq.unitary(circuit), atol=1e-1)
And here are the outputs on my desktop fail-decomposed_circuit.json and on the cloud host pass-decomposed_circuit.json, both outputs are reproducible. The first difference is in the exponent of the first ZPowGate of |
|
@pavoljuhas in the passing ones I see get changed into on the failing ones. This indeed is equal up to global phase, but has a phase difference. My hunch is this is occurring somewhere in |
|
Oh, I think I see. It's odd that it would decompose into entirely different gates on different machines. Maybe it's the |
Changing the default |
BichengYing
pushed a commit
to BichengYing/Cirq
that referenced
this pull request
Jun 20, 2025
* Fix decompose for controlled gates with phase shift * Fix test * Fix type check, int != complex * Remove decomposition to Z * Fix param name * Fix controlled op qasm, reformat tests * Fix test * Fix test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6488.
Fixes #6517.
Fixes the way global phase is handled in decomposition of controlled CZ gates (which are an intermediate step for many decompositions). This PR removes the phase from the Z sub-gate where the existing code had erroneously placed it. Instead, it creates a new, independent controlled-global-phase op as part of the decomposition, with the correct control values, and adds that into the decomposition to compensate.
The PR also adds a handler to allow the controlled global phase op to be further decomposed into Rz's and Y's, via an equivalent diagonal gate, as desired. (
decomposewill do this automatically by default, as is checked by theshould_decompose_to_targetin tests).Finally, it hardens the consistency check in the controlled-gate consistency tests, to ensure that the full decomposition of the gates has a consistent unitary. The existing consistency checks in
cirq.testingonly check single decomposition steps, which is why these bugs were missed. Several of the existing checks would have failed if they had done full decomposition.Tests have been added to check this against different dimensions, different control values including non-product like xor, and with symbolic parameters. Minor bugs were found as part of these tests, in the reprs of global phase gates with symbolic parameters and controlled gates with single-qudit dimensions, and a missing qasm check for qid dimension, and these have been fixed here too.