Fix SabreSwap with classically conditioned gates (#8041) (#8058)

* Fix SabreSwap with classically conditioned gates

The calculation of the expected number of predecessors for a gate to
be considered "resolved" in `SabreSwap` was not accounting for wires
stemming from classical conditions.  The tracking of the _actual_ number
of predecessor requirements satisfied did correctly account for this, so
in certain circumstances the actual count could jump from "too low" to
"too high" without passing through "just right", and the gate would
never get added to the circuit.

* Use `successors` to calculate required predecessors

This unifies the calculation of what is considered a required
predecessor by using the same `SabreSwap._successors` function for both
aspects of the comparison: counting the required predecessors and
counting the actual number of applied predecessors.  This simplifies the
logic, since now an update in one place is sufficient, and the wires are
read directly from the DAG, rather than using assumptions about the
nodes.

* Fix incorrect import

* Fix decremented typo

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 4410a9b)

Co-authored-by: Jake Lishman <jake@binhbar.com>

qiskit/transpiler/passes/routing/sabre_swap.py

@@ -139,7 +139,7 @@ def __init__(
         self.heuristic = heuristic
         self.seed = seed
         self.fake_run = fake_run
-        self.applied_predecessors = None
+        self.required_predecessors = None
         self.qubits_decay = None
         self._bit_indices = None
         self.dist_matrix = None
@@ -188,12 +188,9 @@ def run(self, dag):
         self.qubits_decay = dict.fromkeys(dag.qubits, 1)
 
         # Start algorithm from the front layer and iterate until all gates done.
+        self.required_predecessors = self._build_required_predecessors(dag)
         num_search_steps = 0
         front_layer = dag.front_layer()
-        self.applied_predecessors = defaultdict(int)
-        for _, input_node in dag.input_map.items():
-            for successor in self._successors(input_node, dag):
-                self.applied_predecessors[successor] += 1
 
         while front_layer:
             execute_gate_list = []
@@ -228,7 +225,7 @@ def run(self, dag):
                 for node in execute_gate_list:
                     self._apply_gate(mapped_dag, node, current_layout, canonical_register)
                     for successor in self._successors(node, dag):
-                        self.applied_predecessors[successor] += 1
+                        self.required_predecessors[successor] -= 1
                         if self._is_resolved(successor):
                             front_layer.append(successor)
 
@@ -314,6 +311,15 @@ def _reset_qubits_decay(self):
         """
         self.qubits_decay = {k: 1 for k in self.qubits_decay.keys()}
 
+    def _build_required_predecessors(self, dag):
+        out = defaultdict(int)
+        # We don't need to count in- or out-wires: outs can never be predecessors, and all input
+        # wires are automatically satisfied at the start.
+        for node in dag.op_nodes():
+            for successor in self._successors(node, dag):
+                out[successor] += 1
+        return out
+
     def _successors(self, node, dag):
         """Return an iterable of the successors along each wire from the given node.
 
@@ -326,22 +332,22 @@ def _successors(self, node, dag):
 
     def _is_resolved(self, node):
         """Return True if all of a node's predecessors in dag are applied."""
-        return self.applied_predecessors[node] == len(node.qargs) + len(node.cargs)
+        return self.required_predecessors[node] == 0
 
     def _obtain_extended_set(self, dag, front_layer):
         """Populate extended_set by looking ahead a fixed number of gates.
         For each existing element add a successor until reaching limit.
         """
         extended_set = []
-        incremented = []
+        decremented = []
         tmp_front_layer = front_layer
         done = False
         while tmp_front_layer and not done:
             new_tmp_front_layer = []
             for node in tmp_front_layer:
                 for successor in self._successors(node, dag):
-                    incremented.append(successor)
-                    self.applied_predecessors[successor] += 1
+                    decremented.append(successor)
+                    self.required_predecessors[successor] -= 1
                     if self._is_resolved(successor):
                         new_tmp_front_layer.append(successor)
                         if len(successor.qargs) == 2:
@@ -350,8 +356,8 @@ def _obtain_extended_set(self, dag, front_layer):
                     done = True
                     break
             tmp_front_layer = new_tmp_front_layer
-        for node in incremented:
-            self.applied_predecessors[node] -= 1
+        for node in decremented:
+            self.required_predecessors[node] += 1
         return extended_set
 
     def _obtain_swaps(self, front_layer, current_layout):

releasenotes/notes/sabreswap-fix-condition-593f36e855f9064c.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    The :class:`.SabreSwap` transpiler pass, used in :func:`.transpile` when
+    ``routing_method="sabre"`` is set, will no longer sporadically drop
+    classically conditioned gates and their successors from circuits during the
+    routing phase of transpilation.  See
+    `#8040 <https://github.com/Qiskit/qiskit-terra/issues/8040>`__.

test/python/transpiler/test_sabre_swap.py

@@ -17,9 +17,9 @@
 import ddt
 
 from qiskit.circuit.library import CCXGate, HGate, Measure, SwapGate
-from qiskit.transpiler.passes import SabreSwap
+from qiskit.transpiler.passes import SabreSwap, TrivialLayout
 from qiskit.transpiler import CouplingMap, PassManager
-from qiskit import QuantumRegister, QuantumCircuit
+from qiskit import ClassicalRegister, QuantumRegister, QuantumCircuit
 from qiskit.test import QiskitTestCase
 from qiskit.utils import optionals
 
@@ -237,6 +237,31 @@ def leak_number_of_swaps(cls, *args, **kwargs):
         out_results = sim.run(routed, shots=4096).result().get_counts()
         self.assertEqual(set(in_results), set(out_results))
 
+    def test_classical_condition(self):
+        """Test that :class:`.SabreSwap` correctly accounts for classical conditions in its
+        reckoning on whether a node is resolved or not.  If it is not handled correctly, the second
+        gate might not appear in the output.
+
+        Regression test of gh-8040."""
+        with self.subTest("1 bit in register"):
+            qc = QuantumCircuit(2, 1)
+            qc.z(0)
+            qc.z(0).c_if(qc.cregs[0], 0)
+            cm = CouplingMap([(0, 1), (1, 0)])
+            expected = PassManager([TrivialLayout(cm)]).run(qc)
+            actual = PassManager([TrivialLayout(cm), SabreSwap(cm)]).run(qc)
+            self.assertEqual(expected, actual)
+        with self.subTest("multiple registers"):
+            cregs = [ClassicalRegister(3), ClassicalRegister(4)]
+            qc = QuantumCircuit(QuantumRegister(2, name="q"), *cregs)
+            qc.z(0)
+            qc.z(0).c_if(cregs[0], 0)
+            qc.z(0).c_if(cregs[1], 0)
+            cm = CouplingMap([(0, 1), (1, 0)])
+            expected = PassManager([TrivialLayout(cm)]).run(qc)
+            actual = PassManager([TrivialLayout(cm), SabreSwap(cm)]).run(qc)
+            self.assertEqual(expected, actual)
+
 
 if __name__ == "__main__":
     unittest.main()