Fixed commutation checking between two Pauli product measurements

crates/transpiler/src/commutation_checker.rs

@@ -16,6 +16,7 @@ use ndarray::linalg::kron;
 use num_complex::Complex64;
 use num_complex::ComplexFloat;
 use qiskit_circuit::object_registry::PyObjectAsKey;
+use qiskit_circuit::operations::PauliProductMeasurement;
 use qiskit_circuit::standard_gate::standard_generators::standard_gate_exponent;
 use qiskit_quantum_info::sparse_observable::{PySparseObservable, SparseObservable};
 use smallvec::SmallVec;
@@ -226,6 +227,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],
@@ -496,6 +509,22 @@ impl CommutationChecker {
             _ => (),
         };
 
+        // 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
         let size = qargs1.iter().chain(qargs2.iter()).max().unwrap().0 + 1;

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

@@ -525,10 +525,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"),
@@ -544,10 +545,69 @@ 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."""
@@ -585,7 +645,8 @@ 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_indices = list(range(len(pauli)))
+        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)
@@ -607,7 +668,9 @@ def test_pauli_and_standard_gate(self, pauli_type):
 
         for std_gate, indices, expected in cases:
             with self.subTest(std_gate=std_gate, indices=indices):
-                commutes = scc.commute(pauli_gate, pauli_indices, [], std_gate, indices, [])
+                commutes = scc.commute(
+                    pauli_gate, pauli_qubits, pauli_clbits, std_gate, indices, []
+                )
                 self.assertEqual(expected, commutes)