Don't run routing in SabreLayout with split components (Qiskit#10000)

* Don't run routing in SabreLayout with split components

This commit fixes an issue in the `SabreLayout` pass when run on
backends with a disjoint connectivity graph. The `SabreLayout`
pass internally runs routing as part of its operation and as a
performance efficiency we use that routing output by default. However,
in the case of a disjoint coupling map we are splitting the circuit up
into different subcircuits to map that to the connected components on
the backend. Doing final routing as part of that split context is no
sound because depsite the quantum operations being isolated to each
component, other operations could have a dependency between the
components. This was previously discovered during the development
of Qiskit#9802 to be the case with classical data/bits, but since merging
that it also has come up with barriers. To prevent any issues in the
future this commit changes the SabreLayout pass to never use the routing
output during the layout search when there is >1 connected component
because we *need* to rerun routing on the full circuit after applying
the layout.

Fixes Qiskit#9995

* Remove unused shared_clbits logic

* Remove swap and routing assertions from disjoint sabre layout tests

* Fix lint

Author: mtreinish

qiskit/transpiler/passes/layout/sabre_layout.py

@@ -232,8 +232,6 @@ def run(self, dag):
         )
         initial_layout_dict = {}
         final_layout_dict = {}
-        shared_clbits = False
-        seen_clbits = set()
         for (
             layout_dict,
             final_dict,
@@ -244,20 +242,14 @@ def run(self, dag):
         ) in layout_components:
             initial_layout_dict.update({k: component_map[v] for k, v in layout_dict.items()})
             final_layout_dict.update({component_map[k]: component_map[v] for k, v in final_dict})
-            if not shared_clbits:
-                for clbit in local_dag.clbits:
-                    if clbit in seen_clbits:
-                        shared_clbits = True
-                        break
-                    seen_clbits.add(clbit)
         self.property_set["layout"] = Layout(initial_layout_dict)
         # If skip_routing is set then return the layout in the property set
         # and throwaway the extra work we did to compute the swap map.
-        # We also skip routing here if the input circuit is split over multiple
-        # connected components and there is a shared clbit between any
-        # components. We can only reliably route the full dag if there is any
-        # shared classical data.
-        if self.skip_routing or shared_clbits:
+        # We also skip routing here if there is more than one connected
+        # component we ran layout on. We can only reliably route the full dag
+        # in this case if there is any dependency between the components
+        # (typically shared classical data or barriers).
+        if self.skip_routing or len(layout_components) > 1:
             return dag
         # After this point the pass is no longer an analysis pass and the
         # output circuit returned is transformed with the layout applied

test/python/compiler/test_transpiler.py

@@ -2274,7 +2274,7 @@ def _visit_block(circuit, qubit_mapping=None):
         )
 
     @slow_test
-    @data(1, 2, 3)
+    @data(2, 3)
     def test_six_component_circuit(self, opt_level):
         """Test input circuit with more than 1 component per backend component."""
         qc = QuantumCircuit(42)
@@ -2329,6 +2329,61 @@ def test_six_component_circuit(self, opt_level):
                 continue
             self.assertIn(qubits, self.backend.target[op_name])
 
+    def test_six_component_circuit_level_1(self):
+        """Test input circuit with more than 1 component per backend component."""
+        opt_level = 1
+        qc = QuantumCircuit(42)
+        qc.h(0)
+        qc.h(10)
+        qc.h(20)
+        qc.cx(0, 1)
+        qc.cx(0, 2)
+        qc.cx(0, 3)
+        qc.cx(0, 4)
+        qc.cx(0, 5)
+        qc.cx(0, 6)
+        qc.cx(0, 7)
+        qc.cx(0, 8)
+        qc.cx(0, 9)
+        qc.ecr(10, 11)
+        qc.ecr(10, 12)
+        qc.ecr(10, 13)
+        qc.ecr(10, 14)
+        qc.ecr(10, 15)
+        qc.ecr(10, 16)
+        qc.ecr(10, 17)
+        qc.ecr(10, 18)
+        qc.ecr(10, 19)
+        qc.cy(20, 21)
+        qc.cy(20, 22)
+        qc.cy(20, 23)
+        qc.cy(20, 24)
+        qc.cy(20, 25)
+        qc.cy(20, 26)
+        qc.cy(20, 27)
+        qc.cy(20, 28)
+        qc.cy(20, 29)
+        qc.h(30)
+        qc.cx(30, 31)
+        qc.cx(30, 32)
+        qc.cx(30, 33)
+        qc.h(34)
+        qc.cx(34, 35)
+        qc.cx(34, 36)
+        qc.cx(34, 37)
+        qc.h(38)
+        qc.cx(38, 39)
+        qc.cx(39, 40)
+        qc.cx(39, 41)
+        qc.measure_all()
+        tqc = transpile(qc, self.backend, optimization_level=opt_level, seed_transpiler=42)
+        for inst in tqc.data:
+            qubits = tuple(tqc.find_bit(x).index for x in inst.qubits)
+            op_name = inst.operation.name
+            if op_name == "barrier":
+                continue
+            self.assertIn(qubits, self.backend.target[op_name])
+
     @data(0, 1, 2, 3)
     def test_shared_classical_between_components_condition(self, opt_level):
         """Test a condition sharing classical bits between components."""

test/python/transpiler/test_sabre_layout.py

@@ -241,18 +241,9 @@ def test_dual_ghz(self):
         layout_routing_pass = SabreLayout(
             self.dual_grid_cmap, seed=123456, swap_trials=1, layout_trials=1
         )
-        out = layout_routing_pass(qc)
+        layout_routing_pass(qc)
         layout = layout_routing_pass.property_set["layout"]
         self.assertEqual([layout[q] for q in qc.qubits], [3, 1, 2, 5, 4, 6, 7, 8])
-        self.assertEqual(1, out.count_ops()["swap"])
-        edge_set = set(self.dual_grid_cmap.graph.edge_list())
-        for gate in out.data:
-            if len(gate.qubits) == 2:
-                qubits = tuple(out.find_bit(x).index for x in gate.qubits)
-                # Handle reverse edges which will be fixed by gate direction
-                # later
-                if qubits not in edge_set:
-                    self.assertIn((qubits[1], qubits[0]), edge_set)
 
     def test_dual_ghz_with_wide_barrier(self):
         """Test a basic example with 2 circuit components and 2 cmap components."""
@@ -269,18 +260,9 @@ def test_dual_ghz_with_wide_barrier(self):
         layout_routing_pass = SabreLayout(
             self.dual_grid_cmap, seed=123456, swap_trials=1, layout_trials=1
         )
-        out = layout_routing_pass(qc)
+        layout_routing_pass(qc)
         layout = layout_routing_pass.property_set["layout"]
         self.assertEqual([layout[q] for q in qc.qubits], [3, 1, 2, 5, 4, 6, 7, 8])
-        self.assertEqual(1, out.count_ops()["swap"])
-        edge_set = set(self.dual_grid_cmap.graph.edge_list())
-        for gate in out.data:
-            if len(gate.qubits) == 2:
-                qubits = tuple(out.find_bit(x).index for x in gate.qubits)
-                # Handle reverse edges which will be fixed by gate direction
-                # later
-                if qubits not in edge_set:
-                    self.assertIn((qubits[1], qubits[0]), edge_set)
 
     def test_dual_ghz_with_intermediate_barriers(self):
         """Test dual ghz circuit with intermediate barriers local to each componennt."""
@@ -299,18 +281,9 @@ def test_dual_ghz_with_intermediate_barriers(self):
         layout_routing_pass = SabreLayout(
             self.dual_grid_cmap, seed=123456, swap_trials=1, layout_trials=1
         )
-        out = layout_routing_pass(qc)
+        layout_routing_pass(qc)
         layout = layout_routing_pass.property_set["layout"]
         self.assertEqual([layout[q] for q in qc.qubits], [3, 1, 2, 5, 4, 6, 7, 8])
-        self.assertEqual(1, out.count_ops()["swap"])
-        edge_set = set(self.dual_grid_cmap.graph.edge_list())
-        for gate in out.data:
-            if len(gate.qubits) == 2:
-                qubits = tuple(out.find_bit(x).index for x in gate.qubits)
-                # Handle reverse edges which will be fixed by gate direction
-                # later
-                if qubits not in edge_set:
-                    self.assertIn((qubits[1], qubits[0]), edge_set)
 
     def test_dual_ghz_with_intermediate_spanning_barriers(self):
         """Test dual ghz circuit with barrier in the middle across components."""
@@ -328,18 +301,9 @@ def test_dual_ghz_with_intermediate_spanning_barriers(self):
         layout_routing_pass = SabreLayout(
             self.dual_grid_cmap, seed=123456, swap_trials=1, layout_trials=1
         )
-        out = layout_routing_pass(qc)
+        layout_routing_pass(qc)
         layout = layout_routing_pass.property_set["layout"]
         self.assertEqual([layout[q] for q in qc.qubits], [3, 1, 2, 5, 4, 6, 7, 8])
-        self.assertEqual(1, out.count_ops()["swap"])
-        edge_set = set(self.dual_grid_cmap.graph.edge_list())
-        for gate in out.data:
-            if len(gate.qubits) == 2:
-                qubits = tuple(out.find_bit(x).index for x in gate.qubits)
-                # Handle reverse edges which will be fixed by gate direction
-                # later
-                if qubits not in edge_set:
-                    self.assertIn((qubits[1], qubits[0]), edge_set)
 
     def test_too_large_components(self):
         """Assert trying to run a circuit with too large a connected component raises."""