From 9030922f12eed75ff53793b32feaa87cb3b66afb Mon Sep 17 00:00:00 2001 From: Julien Gacon Date: Thu, 16 Jan 2025 16:42:34 +0100 Subject: [PATCH] Fix 4-pi periodicity of controlled rotations in the `CommutationChecker` (#13670) * start * fix 4pi periodicity of controlled pauli rots * review comments by Elena & Sasha (cherry picked from commit dffc2df684cbda8a727044781250728ec7573c6f) # Conflicts: # crates/accelerate/src/commutation_checker.rs --- crates/accelerate/src/commutation_checker.rs | 60 ++++++++++++++----- ...eriodic-commutations-3b89d1813513f613.yaml | 9 +++ .../circuit/test_commutation_checker.py | 36 +++++------ 3 files changed, 73 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/fix-4pi-periodic-commutations-3b89d1813513f613.yaml diff --git a/crates/accelerate/src/commutation_checker.rs b/crates/accelerate/src/commutation_checker.rs index fe242c73422f..ff2d4f3c9414 100644 --- a/crates/accelerate/src/commutation_checker.rs +++ b/crates/accelerate/src/commutation_checker.rs @@ -34,6 +34,10 @@ use qiskit_circuit::{BitType, Clbit, Qubit}; use crate::unitary_compose; use crate::QiskitError; +<<<<<<< HEAD +======= +// These gates do not commute with other gates, we do not check them. +>>>>>>> dffc2df6 (Fix 4-pi periodicity of controlled rotations in the `CommutationChecker` (#13670)) static SKIPPED_NAMES: [&str; 4] = ["measure", "reset", "delay", "initialize"]; static NO_CACHE_NAMES: [&str; 2] = ["annotated", "linear_function"]; static SUPPORTED_OP: Lazy> = Lazy::new(|| { @@ -43,25 +47,46 @@ static SUPPORTED_OP: Lazy> = Lazy::new(|| { ]) }); +<<<<<<< HEAD const TWOPI: f64 = 2.0 * std::f64::consts::PI; // map rotation gates to their generators, or to ``None`` if we cannot currently efficiently // represent the generator in Rust and store the commutation relation in the commutation dictionary static SUPPORTED_ROTATIONS: Lazy>> = Lazy::new(|| { +======= +// Map rotation gates to their generators (or to ``None`` if we cannot currently efficiently +// represent the generator in Rust and store the commutation relation in the commutation dictionary) +// and their pi-periodicity. Here we mean a gate is n-pi periodic, if for angles that are +// multiples of n*pi, the gate is equal to the identity up to a global phase. +// E.g. RX is generated by X and 2-pi periodic, while CRX is generated by CX and 4-pi periodic. +static SUPPORTED_ROTATIONS: Lazy)>> = Lazy::new(|| { +>>>>>>> dffc2df6 (Fix 4-pi periodicity of controlled rotations in the `CommutationChecker` (#13670)) HashMap::from([ - ("rx", Some(OperationRef::Standard(StandardGate::XGate))), - ("ry", Some(OperationRef::Standard(StandardGate::YGate))), - ("rz", Some(OperationRef::Standard(StandardGate::ZGate))), - ("p", Some(OperationRef::Standard(StandardGate::ZGate))), - ("u1", Some(OperationRef::Standard(StandardGate::ZGate))), - ("crx", Some(OperationRef::Standard(StandardGate::CXGate))), - ("cry", Some(OperationRef::Standard(StandardGate::CYGate))), - ("crz", Some(OperationRef::Standard(StandardGate::CZGate))), - ("cp", Some(OperationRef::Standard(StandardGate::CZGate))), - ("rxx", None), // None means the gate is in the commutation dictionary - ("ryy", None), - ("rzx", None), - ("rzz", None), + ("rx", (2, Some(OperationRef::Standard(StandardGate::XGate)))), + ("ry", (2, Some(OperationRef::Standard(StandardGate::YGate)))), + ("rz", (2, Some(OperationRef::Standard(StandardGate::ZGate)))), + ("p", (2, Some(OperationRef::Standard(StandardGate::ZGate)))), + ("u1", (2, Some(OperationRef::Standard(StandardGate::ZGate)))), + ("rxx", (2, None)), // None means the gate is in the commutation dictionary + ("ryy", (2, None)), + ("rzx", (2, None)), + ("rzz", (2, None)), + ( + "crx", + (4, Some(OperationRef::Standard(StandardGate::CXGate))), + ), + ( + "cry", + (4, Some(OperationRef::Standard(StandardGate::CYGate))), + ), + ( + "crz", + (4, Some(OperationRef::Standard(StandardGate::CZGate))), + ), + ( + "cp", + (2, Some(OperationRef::Standard(StandardGate::CZGate))), + ), ]) }); @@ -629,12 +654,19 @@ fn map_rotation<'a>( tol: f64, ) -> (&'a OperationRef<'a>, &'a [Param], bool) { let name = op.name(); +<<<<<<< HEAD if let Some(generator) = SUPPORTED_ROTATIONS.get(name) { // if the rotation angle is below the tolerance, the gate is assumed to +======= + + if let Some((pi_multiple, generator)) = SUPPORTED_ROTATIONS.get(name) { + // If the rotation angle is below the tolerance, the gate is assumed to +>>>>>>> dffc2df6 (Fix 4-pi periodicity of controlled rotations in the `CommutationChecker` (#13670)) // commute with everything, and we simply return the operation with the flag that // it commutes trivially if let Param::Float(angle) = params[0] { - if (angle % TWOPI).abs() < tol { + let periodicity = (*pi_multiple as f64) * ::std::f64::consts::PI; + if (angle % periodicity).abs() < tol { return (op, params, true); }; }; diff --git a/releasenotes/notes/fix-4pi-periodic-commutations-3b89d1813513f613.yaml b/releasenotes/notes/fix-4pi-periodic-commutations-3b89d1813513f613.yaml new file mode 100644 index 000000000000..289c43cb422f --- /dev/null +++ b/releasenotes/notes/fix-4pi-periodic-commutations-3b89d1813513f613.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + The :class:`.CommutationChecker` did not handle commutations of the :class:`.CRXGate`, + :class:`.CRYGate` and :class:`.CRZGate` correctly for angles + :math:`\pi(4k + 2)` for :math:`k \in \mathbb Z`. + In these cases, the controlled rotations were falsely assumed to commute with any gate. + Now these gates correctly commute with any gate if the rotation angle is a multiple of + :math:`4\pi`. diff --git a/test/python/circuit/test_commutation_checker.py b/test/python/circuit/test_commutation_checker.py index 9759b5bffd1e..95a1db594475 100644 --- a/test/python/circuit/test_commutation_checker.py +++ b/test/python/circuit/test_commutation_checker.py @@ -53,6 +53,7 @@ XGate, ZGate, HGate, + UnitaryGate, ) from qiskit.dagcircuit import DAGOpNode @@ -61,14 +62,14 @@ RYGate, RZGate, PhaseGate, - CRXGate, - CRYGate, - CRZGate, - CPhaseGate, RXXGate, RYYGate, RZZGate, RZXGate, + CRXGate, + CRYGate, + CRZGate, + CPhaseGate, ] @@ -410,26 +411,25 @@ def test_cutoff_angles(self, gate_cls): self.assertFalse(scc.commute(generic_gate, [0, 1], [], gate, qargs, [])) @idata(ROTATION_GATES) - def test_rotation_mod_2pi(self, gate_cls): - """Test the rotations modulo 2pi commute with any gate.""" + def test_controlled_rotation_mod_4pi(self, gate_cls): + """Test the rotations modulo 2pi (4pi for controlled-rx/y/z) commute with any gate.""" generic_gate = HGate() # does not commute with any rotation gate - even = np.arange(-6, 7, 2) + multiples = np.arange(-6, 7) - with self.subTest(msg="even multiples"): - for multiple in even: + for multiple in multiples: + with self.subTest(multiple=multiple): gate = gate_cls(multiple * np.pi) - self.assertTrue( - scc.commute(generic_gate, [0], [], gate, list(range(gate.num_qubits)), []) - ) + numeric = UnitaryGate(gate.to_matrix()) - odd = np.arange(-5, 6, 2) - with self.subTest(msg="odd multiples"): - for multiple in odd: - gate = gate_cls(multiple * np.pi) - self.assertFalse( - scc.commute(generic_gate, [0], [], gate, list(range(gate.num_qubits)), []) + # compute a numeric reference, that doesn't go through any special cases and + # uses a matrix-based commutation check + expected = scc.commute( + generic_gate, [0], [], numeric, list(range(gate.num_qubits)), [] ) + result = scc.commute(generic_gate, [0], [], gate, list(range(gate.num_qubits)), []) + self.assertEqual(expected, result) + if __name__ == "__main__": unittest.main()