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

* 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

* fix conflicts

* remove comment

---------

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>

Author: mergify[bot]

crates/transpiler/src/commutation_checker.rs

@@ -16,8 +16,8 @@ use ndarray::linalg::kron;
 use num_complex::Complex64;
 use num_complex::ComplexFloat;
 use qiskit_circuit::object_registry::PyObjectAsKey;
-use qiskit_quantum_info::sparse_observable::PySparseObservable;
-use qiskit_quantum_info::sparse_observable::SparseObservable;
+use qiskit_circuit::operations::PauliProductMeasurement;
+use qiskit_quantum_info::sparse_observable::{PySparseObservable, SparseObservable};
 use smallvec::SmallVec;
 use std::fmt::Debug;
 
@@ -220,6 +220,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,6 +533,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 in between Pauli-based gates, like PauliGate or PauliEvolutionGate
         let size = qargs1.iter().chain(qargs2.iter()).max().unwrap().0 + 1;
         if let Some(obs1) = try_pauli_generator(op1, qargs1, 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."""