Fixed commutation checking between two Pauli product measurements (#16023)

* Fix commutation checking between two PPMs

* fix for the case qubits are different

* additional fixes and improvements following the review

* improve test formatting

* formatting (again)

(cherry picked from commit 89ff976)

# Conflicts:
#	crates/transpiler/src/commutation_checker.rs
#	test/python/circuit/test_commutation_checker.py

Author: alexanderivrii

crates/transpiler/src/commutation_checker.rs

@@ -16,8 +16,14 @@ use ndarray::linalg::kron;
 use num_complex::Complex64;
 use num_complex::ComplexFloat;
 use qiskit_circuit::object_registry::PyObjectAsKey;
+<<<<<<< HEAD
 use qiskit_quantum_info::sparse_observable::PySparseObservable;
 use qiskit_quantum_info::sparse_observable::SparseObservable;
+=======
+use qiskit_circuit::operations::PauliProductMeasurement;
+use qiskit_circuit::standard_gate::standard_generators::standard_gate_exponent;
+use qiskit_quantum_info::sparse_observable::{PySparseObservable, SparseObservable};
+>>>>>>> 89ff976e0 (Fixed commutation checking between two Pauli product measurements (#16023))
 use smallvec::SmallVec;
 use std::fmt::Debug;
 
@@ -220,6 +226,18 @@ fn try_extract_op_from_ppm(
     Some(out.compose_map(&local, |i| qubits[i as usize].0))
 }
 
+/// Given a pauli product measurement, returns its generator (represented as a sparse observable)
+/// and the sign (representing the pauli phase).
+fn observable_generator_from_ppm(
+    ppm: &PauliProductMeasurement,
+    qubits: &[Qubit],
+    num_qubits: u32,
+) -> (SparseObservable, bool) {
+    let local = xz_to_observable(&ppm.x, &ppm.z);
+    let out = SparseObservable::identity(num_qubits);
+    (out.compose_map(&local, |i| qubits[i as usize].0), ppm.neg)
+}
+
 fn try_extract_op_from_ppr(
     operation: &OperationRef,
     qubits: &[Qubit],
@@ -521,7 +539,28 @@ impl CommutationChecker {
             _ => (),
         };
 
+<<<<<<< HEAD
         // Handle commutations in between Pauli-based gates, like PauliGate or PauliEvolutionGate
+=======
+        // Special handling for commutativity of two pauli product measurements in the case they write to
+        // the same classical bit. In this case, it's generally incorrect to interchange them, so we only
+        // do this if they have the same generators (represented as sparse observables + signs).
+        if let (
+            OperationRef::PauliProductMeasurement(ppm1),
+            OperationRef::PauliProductMeasurement(ppm2),
+        ) = (op1, op2)
+        {
+            if cargs1 == cargs2 {
+                let size = qargs1.iter().chain(qargs2.iter()).max().unwrap().0 + 1;
+                let pauli1 = observable_generator_from_ppm(ppm1, qargs1, size);
+                let pauli2 = observable_generator_from_ppm(ppm2, qargs2, size);
+                return Ok(pauli1 == pauli2);
+            }
+        }
+
+        // Handle commutations between Pauli-based gates among themselves, and with standard gates
+        // TODO Support trivial commutations of standard gates with identities in the Paulis
+>>>>>>> 89ff976e0 (Fixed commutation checking between two Pauli product measurements (#16023))
         let size = qargs1.iter().chain(qargs2.iter()).max().unwrap().0 + 1;
         if let Some(obs1) = try_pauli_generator(op1, qargs1, size) {
             if let Some(obs2) = try_pauli_generator(op2, qargs2, size) {

releasenotes/notes/fix-ppm-commutation-ddcde0bf8b20a9f8.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixed commutativity check between two :class:`.PauliProductMeasurement` instructions
+    in the case that both instructions measure to the same classical bit. In this case,
+    the later measurement overwrites the result of the earlier measurement, and consequently,
+    interchanging the two measurement instructions inside the quantum circuit is generally
+    invalid.

test/python/circuit/test_commutation_checker.py

@@ -534,10 +534,11 @@ def test_pauli_based_gates(self, gate_type):
         for p1, q1, p2, q2, expected in cases:
             if p1 == "I" and gate_type == "measure":
                 continue  # PPM doesn't support all-identity gates
+            c1, c2 = ([0], [1]) if gate_type == "measure" else ([], [])
 
             gate1 = build_pauli_gate(p1, gate_type)
             gate2 = build_pauli_gate(p2, gate_type)
-            self.assertEqual(expected, scc.commute(gate1, q1, [], gate2, q2, []))
+            self.assertEqual(expected, scc.commute(gate1, q1, c1, gate2, q2, c2))
 
     @data(
         ("pauli", "measure"),
@@ -553,10 +554,45 @@ def test_mix_pauli_gates(self, gate_type1, gate_type2):
         ]
 
         for p1, q1, p2, q2, expected in cases:
+            c1 = [0] if gate_type1 == "measure" else []
+            c2 = [1] if gate_type2 == "measure" else []
+
             gate1 = build_pauli_gate(p1, gate_type1)
             gate2 = build_pauli_gate(p2, gate_type2)
+
             with self.subTest(p1=p1, p2=p2):
-                self.assertEqual(expected, scc.commute(gate1, q1, [], gate2, q2, []))
+                self.assertEqual(expected, scc.commute(gate1, q1, c1, gate2, q2, c2))
+
+    def test_ppms_with_same_clbit(self):
+        """Test commutativity of two Pauli product measurements with the same clbit."""
+
+        # Each case represents (pauli1, qubits1, pauli2, qubits2, expected result).
+        # Recall that the convention is that pauli strings are read right-to-left, i.e.
+        # pauli strings and qubits are in reverse order relative to each other.
+        cases = [
+            # different Paulis
+            ("XXII", [1, 0, 2, 3], "IIZZ", [1, 0, 2, 3], False),
+            # different qubits
+            ("XYIZ", [1, 0, 2, 3], "XYIZ", [1, 0, 4, 3], False),
+            # different Paulis (including sign)
+            ("ZXII", [1, 0, 2, 3], "-ZXII", [1, 0, 2, 3], False),
+            # same Paulis and qubits
+            ("XYIZ", [1, 0, 2, 3], "XYIZ", [1, 0, 2, 3], True),
+            # same Paulis and qubits
+            ("-XYIZ", [1, 0, 2, 3], "-XYIZ", [1, 0, 2, 3], True),
+            # same Paulis and qubits up to reordering
+            ("XXIY", [0, 1, 2, 3], "YIXX", [3, 2, 1, 0], True),
+            # same Paulis and qubits up to reordering
+            ("XXIY", [0, 1, 2, 3], "XXIY", [0, 1, 3, 2], True),
+            # same Paulis and qubits up to reordering
+            ("-XXIY", [0, 1, 2, 3], "-YIXX", [2, 3, 1, 0], True),
+        ]
+
+        for pauli1, qubits1, pauli2, qubits2, expected in cases:
+            with self.subTest(pauli1=pauli1, qubits1=qubits1, pauli2=pauli2, qubits2=qubits2):
+                ppm1 = build_pauli_gate(pauli1, "measure")
+                ppm2 = build_pauli_gate(pauli2, "measure")
+                self.assertEqual(scc.commute(ppm1, qubits1, [0], ppm2, qubits2, [0]), expected)
 
     def test_pauli_evolution_sums(self):
         """Test PauliEvolutionGate commutations for operators that are sums of Paulis."""
@@ -589,6 +625,42 @@ def test_pauli_evolution_parameterized(self):
         with self.subTest(left=z, right=x):
             self.assertFalse(scc.commute(z, qargs, [], x, qargs, []))
 
+<<<<<<< HEAD
+=======
+    @data("evolution", "pauli", "measure", "rotation")
+    def test_pauli_and_standard_gate(self, pauli_type):
+        """Test Pauli-based gates and standard gate commutations are efficiently supported."""
+        # 40-qubit Pauli gate with following terms: X: 0-9, Y: 10-19, Z: 20-29, I: 30-39
+        pauli = 10 * "I" + 10 * "Z" + 10 * "Y" + 10 * "X"
+        pauli_qubits = list(range(len(pauli)))
+        pauli_clbits = [0] if pauli_type == "measure" else []
+        pauli_gate = build_pauli_gate(pauli, pauli_type)
+
+        # Test cases in the format: (gate, indices, commutes)
+        x = Parameter("x")
+        cases = [
+            (CXGate(), [30, 0], True),  # CX vs. IX
+            (CXGate(), [29, 0], True),  # CX vs. ZX
+            (CXGate(), [29, 10], False),  # CX vs. ZY
+            (RXXGate(x), [0, 1], True),
+            (RXXGate(1.2), [11, 15], True),
+            (RXXGate(x), [0, 15], False),
+            (HGate(), [33], True),
+            (HGate(), [2], False),
+            (CPhaseGate(0.1), [30, 31], True),
+            (CPhaseGate(0.1), [2, 3], False),
+            (XXPlusYYGate(1, 1), [0, 1], False),
+            (XXPlusYYGate(1, 0), [0, 1], True),
+        ]
+
+        for std_gate, indices, expected in cases:
+            with self.subTest(std_gate=std_gate, indices=indices):
+                commutes = scc.commute(
+                    pauli_gate, pauli_qubits, pauli_clbits, std_gate, indices, []
+                )
+                self.assertEqual(expected, commutes)
+
+>>>>>>> 89ff976e0 (Fixed commutation checking between two Pauli product measurements (#16023))
 
 def build_pauli_gate(pauli_string: str, gate_type: str) -> Gate:
     """Build a Pauli-based gate off a Pauli string.