Fix bad M{XX,YY,ZZ} tracking due to disjoint decomposition (#723)

- The `stim::decompose_pair_instruction_into_segments_with_single_use_controls` was not guaranteeing the invariants actually used by code calling the method - The detecting regions of circuits with MXX/MYY/MZZ operations were often wrong if the same qubit was used multiple times in the same instruction - Refactored `stim::decompose_pair_instruction_into_segments_with_single_use_controls` into `stim::decompose_pair_instruction_into_disjoint_segments` - Also fixed a hard-to-reproduce issue where the hyper search could use the same error twice
quantumlib · Mar 20, 2024 · 139007a · 139007a
1 parent ebf7320
commit 139007a
Show file tree

Hide file tree

Showing 14 changed files with 110 additions and 60 deletions.
diff --git a/doc/python_api_reference_vDev.md b/doc/python_api_reference_vDev.md
@@ -2356,8 +2356,7 @@ def likeliest_error_sat_problem(
     quantization: int = 100,
     format: str = 'WDIMACS',
 ) -> str:
-    """Makes a maxSAT problem of the circuit's most likely undetectable logical
-    error, that other tools can solve.
+    """Makes a maxSAT problem for the circuit's likeliest undetectable logical error.
 
     The output is a string describing the maxSAT problem in WDIMACS format
     (see https://maxhs.org/docs/wdimacs.html). The optimal solution to the

diff --git a/doc/stim.pyi b/doc/stim.pyi
@@ -1724,8 +1724,7 @@ class Circuit:
         quantization: int = 100,
         format: str = 'WDIMACS',
     ) -> str:
-        """Makes a maxSAT problem of the circuit's most likely undetectable logical
-        error, that other tools can solve.
+        """Makes a maxSAT problem for the circuit's likeliest undetectable logical error.
 
         The output is a string describing the maxSAT problem in WDIMACS format
         (see https://maxhs.org/docs/wdimacs.html). The optimal solution to the

diff --git a/glue/python/src/stim/__init__.pyi b/glue/python/src/stim/__init__.pyi
@@ -1724,8 +1724,7 @@ class Circuit:
         quantization: int = 100,
         format: str = 'WDIMACS',
     ) -> str:
-        """Makes a maxSAT problem of the circuit's most likely undetectable logical
-        error, that other tools can solve.
+        """Makes a maxSAT problem for the circuit's likeliest undetectable logical error.
 
         The output is a string describing the maxSAT problem in WDIMACS format
         (see https://maxhs.org/docs/wdimacs.html). The optimal solution to the

diff --git a/src/stim/circuit/circuit.pybind.cc b/src/stim/circuit/circuit.pybind.cc
@@ -2137,8 +2137,7 @@ void stim_pybind::pybind_circuit_methods(pybind11::module &, pybind11::class_<Ci
         pybind11::arg("quantization") = 100,
         pybind11::arg("format") = "WDIMACS",
         clean_doc_string(R"DOC(
-            Makes a maxSAT problem of the circuit's most likely undetectable logical
-            error, that other tools can solve.
+            Makes a maxSAT problem for the circuit's likeliest undetectable logical error.
 
             The output is a string describing the maxSAT problem in WDIMACS format
             (see https://maxhs.org/docs/wdimacs.html). The optimal solution to the

diff --git a/src/stim/circuit/circuit_pybind_test.py b/src/stim/circuit/circuit_pybind_test.py
@@ -1794,3 +1794,19 @@ def test_detecting_region_filters():
     assert len(c.detecting_regions(targets=["D0"])) == 1
     assert len(c.detecting_regions(targets=["D0", "L0"])) == 2
     assert len(c.detecting_regions(targets=[stim.target_relative_detector_id(0), "D0"])) == 1
+
+
+def test_detecting_regions_mzz():
+    c = stim.Circuit("""
+        TICK
+        MZZ 0 1 1 2
+        TICK
+        M 2
+        DETECTOR rec[-1]
+    """)
+    assert c.detecting_regions() == {
+        stim.target_relative_detector_id(0): {
+            0: stim.PauliString("__Z"),
+            1: stim.PauliString("__Z"),
+        },
+    }
diff --git a/src/stim/circuit/gate_decomposition.cc b/src/stim/circuit/gate_decomposition.cc
@@ -269,30 +269,32 @@ void stim::decompose_spp_or_spp_dag_operation(
     }
 }
 
-void stim::decompose_pair_instruction_into_segments_with_single_use_controls(
+void stim::decompose_pair_instruction_into_disjoint_segments(
     const CircuitInstruction &inst, size_t num_qubits, const std::function<void(CircuitInstruction)> &callback) {
-    simd_bits<64> used_as_control(std::max(num_qubits, size_t{1}));
-    size_t done = 0;
-    size_t k = 0;
-    while (done < inst.targets.size()) {
-        bool flush = true;
-        size_t q0 = 0;
-        if (k < inst.targets.size()) {
-            q0 = inst.targets[k].qubit_value();
-            size_t q1 = inst.targets[k + 1].qubit_value();
-            flush = used_as_control[q0] || used_as_control[q1];
-        }
-        if (flush) {
-            callback(CircuitInstruction{
-                inst.gate_type,
-                inst.args,
-                inst.targets.sub(done, k),
-            });
-            used_as_control.clear();
-            done = k;
+    simd_bits<64> used_as_control(num_qubits);
+    size_t num_flushed = 0;
+    size_t cur_index = 0;
+    auto flush = [&](){
+        callback(CircuitInstruction{
+            inst.gate_type,
+            inst.args,
+            inst.targets.sub(num_flushed, cur_index),
+        });
+        used_as_control.clear();
+        num_flushed = cur_index;
+    };
+    while (cur_index < inst.targets.size()) {
+        size_t q0 = inst.targets[cur_index].qubit_value();
+        size_t q1 = inst.targets[cur_index + 1].qubit_value();
+        if (used_as_control[q0] || used_as_control[q1]) {
+            flush();
         }
         used_as_control[q0] = true;
-        k += 2;
+        used_as_control[q1] = true;
+        cur_index += 2;
+    }
+    if (num_flushed < inst.targets.size()) {
+        flush();
     }
 }
 

diff --git a/src/stim/circuit/gate_decomposition.h b/src/stim/circuit/gate_decomposition.h
@@ -96,7 +96,7 @@ void decompose_spp_or_spp_dag_operation(
 ///         instruction must be less than this.
 ///     inst: The circuit instruction to decompose.
 ///     callback: The method called with each decomposed segment.
-void decompose_pair_instruction_into_segments_with_single_use_controls(
+void decompose_pair_instruction_into_disjoint_segments(
     const CircuitInstruction &inst, size_t num_qubits, const std::function<void(CircuitInstruction)> &callback);
 
 bool accumulate_next_obs_terms_to_pauli_string_helper(

diff --git a/src/stim/circuit/gate_decomposition.test.cc b/src/stim/circuit/gate_decomposition.test.cc
@@ -124,36 +124,44 @@ TEST(gate_decomposition, decompose_mpp_to_mpad) {
         std::invalid_argument);
 }
 
-TEST(gate_decomposition, decompose_pair_instruction_into_segments_with_single_use_controls) {
+TEST(gate_decomposition, decompose_pair_instruction_into_disjoint_segments) {
     Circuit out;
     auto append_into_circuit = [&](const CircuitInstruction &segment) {
-        std::vector<GateTarget> evens;
-        for (size_t k = 0; k < segment.targets.size(); k += 2) {
-            evens.push_back(segment.targets[k]);
-        }
-        out.safe_append(CircuitInstruction{GateType::CX, {}, segment.targets});
-        out.safe_append(CircuitInstruction{GateType::MX, segment.args, evens});
-        out.safe_append(CircuitInstruction{GateType::CX, {}, segment.targets});
+        out.safe_append(segment);
         out.append_from_text("TICK");
     };
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         Circuit("MXX(0.125) 0 1 0 2 3 5 4 5 3 4").operations[0], 10, append_into_circuit);
     ASSERT_EQ(out, Circuit(R"CIRCUIT(
-        CX 0 1
-        MX(0.125) 0
-        CX 0 1
+        MXX(0.125) 0 1
+        TICK
+
+        MXX(0.125) 0 2 3 5
+        TICK
+
+        MXX(0.125) 4 5
+        TICK
+
+        MXX(0.125) 3 4
         TICK
+    )CIRCUIT"));
 
-        CX 0 2 3 5 4 5
-        MX(0.125) 0 3 4
-        CX 0 2 3 5 4 5
+    out.clear();
+    decompose_pair_instruction_into_disjoint_segments(
+        Circuit("MZZ 0 1 1 2").operations[0], 10, append_into_circuit);
+    ASSERT_EQ(out, Circuit(R"CIRCUIT(
+        MZZ 0 1
         TICK
 
-        CX 3 4
-        MX(0.125) 3
-        CX 3 4
+        MZZ 1 2
         TICK
     )CIRCUIT"));
+
+    out.clear();
+    decompose_pair_instruction_into_disjoint_segments(
+        Circuit("MZZ").operations[0], 10, append_into_circuit);
+    ASSERT_EQ(out, Circuit(R"CIRCUIT(
+    )CIRCUIT"));
 }
 
 TEST(gate_decomposition, decompose_spp_or_spp_dag_operation_simple) {

diff --git a/src/stim/search/hyper/algo.cc b/src/stim/search/hyper/algo.cc
@@ -39,6 +39,16 @@ DetectorErrorModel backtrack_path(const std::map<SearchState, SearchState> &back
         cur_state = prev_state;
     }
     std::sort(out.instructions.begin(), out.instructions.end());
+
+    // Because of the search truncation, the same step may have been taken twice at different states.
+    for (size_t k = 0; k < out.instructions.size() - 1; k++) {
+        if (out.instructions[k].target_data == out.instructions[k + 1].target_data) {
+            out.instructions.erase(out.instructions.begin() + k);
+            out.instructions.erase(out.instructions.begin() + k);
+            k -= 1;
+        }
+    }
+
     return out;
 }
 

diff --git a/src/stim/simulators/error_analyzer.cc b/src/stim/simulators/error_analyzer.cc
@@ -1677,7 +1677,7 @@ void ErrorAnalyzer::undo_MXX(const CircuitInstruction &inst) {
         reversed_targets[k] = inst.targets[n - k - 1];
     }
 
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         {inst.gate_type, inst.args, reversed_targets}, tracker.xs.size(), [&](CircuitInstruction segment) {
             undo_MXX_disjoint_controls_segment(segment);
         });
@@ -1691,7 +1691,7 @@ void ErrorAnalyzer::undo_MYY(const CircuitInstruction &inst) {
         reversed_targets[k] = inst.targets[n - k - 1];
     }
 
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         {inst.gate_type, inst.args, reversed_targets}, tracker.xs.size(), [&](CircuitInstruction segment) {
             undo_MYY_disjoint_controls_segment(segment);
         });
@@ -1705,7 +1705,7 @@ void ErrorAnalyzer::undo_MZZ(const CircuitInstruction &inst) {
         reversed_targets[k] = inst.targets[n - k - 1];
     }
 
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         {inst.gate_type, inst.args, reversed_targets}, tracker.xs.size(), [&](CircuitInstruction segment) {
             undo_MZZ_disjoint_controls_segment(segment);
         });

diff --git a/src/stim/simulators/frame_simulator.inl b/src/stim/simulators/frame_simulator.inl
@@ -882,23 +882,23 @@ void FrameSimulator<W>::do_MZZ_disjoint_controls_segment(const CircuitInstructio
 
 template <size_t W>
 void FrameSimulator<W>::do_MXX(const CircuitInstruction &inst) {
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         inst, num_qubits, [&](CircuitInstruction segment) {
             do_MXX_disjoint_controls_segment(segment);
         });
 }
 
 template <size_t W>
 void FrameSimulator<W>::do_MYY(const CircuitInstruction &inst) {
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         inst, num_qubits, [&](CircuitInstruction segment) {
             do_MYY_disjoint_controls_segment(segment);
         });
 }
 
 template <size_t W>
 void FrameSimulator<W>::do_MZZ(const CircuitInstruction &inst) {
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         inst, num_qubits, [&](CircuitInstruction segment) {
             do_MZZ_disjoint_controls_segment(segment);
         });

diff --git a/src/stim/simulators/sparse_rev_frame_tracker.cc b/src/stim/simulators/sparse_rev_frame_tracker.cc
@@ -474,7 +474,7 @@ void SparseUnsignedRevFrameTracker::undo_MXX(const CircuitInstruction &inst) {
         reversed_targets[k] = inst.targets[n - k - 1];
     }
 
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         {inst.gate_type, inst.args, reversed_targets}, xs.size(), [&](CircuitInstruction segment) {
             undo_MXX_disjoint_controls_segment(segment);
         });
@@ -488,7 +488,7 @@ void SparseUnsignedRevFrameTracker::undo_MYY(const CircuitInstruction &inst) {
         reversed_targets[k] = inst.targets[n - k - 1];
     }
 
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         {inst.gate_type, inst.args, reversed_targets}, xs.size(), [&](CircuitInstruction segment) {
             undo_MYY_disjoint_controls_segment(segment);
         });
@@ -502,7 +502,7 @@ void SparseUnsignedRevFrameTracker::undo_MZZ(const CircuitInstruction &inst) {
         reversed_targets[k] = inst.targets[n - k - 1];
     }
 
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         {inst.gate_type, inst.args, reversed_targets}, xs.size(), [&](CircuitInstruction segment) {
             undo_MZZ_disjoint_controls_segment(segment);
         });

diff --git a/src/stim/simulators/sparse_rev_frame_tracker.test.cc b/src/stim/simulators/sparse_rev_frame_tracker.test.cc
@@ -712,3 +712,21 @@ TEST(SparseUnsignedRevFrameTracker, tracks_anticommutation) {
     SparseUnsignedRevFrameTracker rev2(circuit.count_qubits(), circuit.count_measurements(), circuit.count_detectors());
     ASSERT_THROW({ rev.undo_circuit(circuit); }, std::invalid_argument);
 }
+
+TEST(SparseUnsignedRevFrameTracker, MZZ) {
+    Circuit circuit(R"CIRCUIT(
+        MZZ 0 1 1 2
+        M 2
+        DETECTOR rec[-1]
+    )CIRCUIT");
+
+    SparseUnsignedRevFrameTracker rev(
+        circuit.count_qubits(), circuit.count_measurements(), circuit.count_detectors(), false);
+    rev.undo_circuit(circuit);
+    ASSERT_TRUE(rev.xs[0].empty());
+    ASSERT_TRUE(rev.xs[1].empty());
+    ASSERT_TRUE(rev.xs[2].empty());
+    ASSERT_TRUE(rev.zs[0].empty());
+    ASSERT_TRUE(rev.zs[1].empty());
+    ASSERT_EQ(rev.zs[2].sorted_items, (std::vector<DemTarget>{DemTarget::relative_detector_id(0)}));
+}
diff --git a/src/stim/simulators/tableau_simulator.inl b/src/stim/simulators/tableau_simulator.inl
@@ -312,23 +312,23 @@ void TableauSimulator<W>::do_MZZ_disjoint_controls_segment(const CircuitInstruct
 
 template <size_t W>
 void TableauSimulator<W>::do_MXX(const CircuitInstruction &inst) {
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         inst, inv_state.num_qubits, [&](CircuitInstruction segment) {
             do_MXX_disjoint_controls_segment(segment);
         });
 }
 
 template <size_t W>
 void TableauSimulator<W>::do_MYY(const CircuitInstruction &inst) {
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         inst, inv_state.num_qubits, [&](CircuitInstruction segment) {
             do_MYY_disjoint_controls_segment(segment);
         });
 }
 
 template <size_t W>
 void TableauSimulator<W>::do_MZZ(const CircuitInstruction &inst) {
-    decompose_pair_instruction_into_segments_with_single_use_controls(
+    decompose_pair_instruction_into_disjoint_segments(
         inst, inv_state.num_qubits, [&](CircuitInstruction segment) {
             do_MZZ_disjoint_controls_segment(segment);
         });