Ensure we run VF2PostLayout when needed in optimization level 1

qiskit/transpiler/passes/utils/check_map.py

@@ -22,19 +22,27 @@ class CheckMap(AnalysisPass):
 
     Check if a DAGCircuit is mapped to `coupling_map` by checking that all
     2-qubit interactions are laid out to be on adjacent qubits in the global coupling
-    map of the device, setting the property ``is_swap_mapped`` to ``True`` or ``False``
-    accordingly. Note this does not validate directionality of the connectivity between
-    qubits. If you need to check gates are implemented in a native direction
-    for a target use the :class:`~.CheckGateDirection` pass instead.
+    map of the device, setting the property set field (either specified with ``property_set_field``
+    or the default ``is_swap_mapped``) to ``True`` or ``False`` accordingly. Note this does not
+    validate directionality of the connectivity between  qubits. If you need to check gates are
+    implemented in a native direction for a target use the :class:`~.CheckGateDirection` pass
+    instead.
     """
 
-    def __init__(self, coupling_map):
+    def __init__(self, coupling_map, property_set_field=None):
         """CheckMap initializer.
 
         Args:
             coupling_map (Union[CouplingMap, Target]): Directed graph representing a coupling map.
+            property_set_field (str): An optional string to specify the property set field to
+                store the result of the check. If not default the result is stored in
+                ``"is_swap_mapped"``.
         """
         super().__init__()
+        if property_set_field is None:
+            self.property_set_field = "is_swap_mapped"
+        else:
+            self.property_set_field = property_set_field
         if isinstance(coupling_map, Target):
             cmap = coupling_map.build_coupling_map()
         else:
@@ -58,7 +66,7 @@ def run(self, dag):
         """
         from qiskit.converters import circuit_to_dag
 
-        self.property_set["is_swap_mapped"] = True
+        self.property_set[self.property_set_field] = True
 
         if not self.qargs:
             return
@@ -77,7 +85,7 @@ def run(self, dag):
                         physical_q0,
                         physical_q1,
                     )
-                    self.property_set["is_swap_mapped"] = False
+                    self.property_set[self.property_set_field] = False
                     return
             elif is_controlflow_op:
                 order = [qubit_indices[bit] for bit in node.qargs]
@@ -86,5 +94,5 @@ def run(self, dag):
                     mapped_dag = dag.copy_empty_like()
                     mapped_dag.compose(dag_block, qubits=order)
                     self.run(mapped_dag)
-                    if not self.property_set["is_swap_mapped"]:
+                    if not self.property_set[self.property_set_field]:
                         return

qiskit/transpiler/preset_passmanagers/common.py

@@ -274,12 +274,12 @@ def _run_post_layout_condition(property_set):
 
     routing = PassManager()
     if target is not None:
-        routing.append(CheckMap(target))
+        routing.append(CheckMap(target, property_set_field="routing_not_needed"))
     else:
-        routing.append(CheckMap(coupling_map))
+        routing.append(CheckMap(coupling_map, property_set_field="routing_not_needed"))
 
     def _swap_condition(property_set):
-        return not property_set["is_swap_mapped"]
+        return not property_set["routing_not_needed"]
 
     if use_barrier_before_measurement:
         routing.append([BarrierBeforeFinalMeasurements(), routing_pass], condition=_swap_condition)

releasenotes/notes/vf2postlayout-fix-16bb54d9bdf3aaf6.yaml

@@ -0,0 +1,23 @@
+---
+features:
+  - |
+    The :class:~.CheckMap` transpiler pass has a new keyword argument on its
+    constructor, ``property_set_field``. This argument can be used to specify
+    a field in the property set to store the results of the analysis.
+    Previously, it was only possible to store the result in the field
+    ``"is_swap_mapped"`` (which is the default). This enables you to store the
+    result of multiple instances of the pass in a :class:`~.PassManager` in
+    different fields.
+fixes:
+  - |
+    Fixed an issue in :func:`~.transpile` with ``optimization_level=1`` (as
+    well as in the preset pass managers returned by
+    :func:`~.generate_preset_pass_manager` and :func:`~.level_1_pass_manager`)
+    where previously if the ``routing_method`` and ``layout_method`` arguments
+    were not set and no control flow operations were present in the circuit
+    then in cases where routing was required the
+    :class:`~.VF2PostLayout` transpiler pass would not be run. This was the
+    opposite of the expected behavior because :class:`~.VF2PostLayout` is
+    intended to find a potentially better performing layout after a heuristic
+    layout pass and routing are run.
+    Fixed `#9936 <https://github.com/Qiskit/qiskit-terra/issues/9936>`__

test/python/transpiler/test_preset_passmanagers.py

@@ -20,7 +20,7 @@
 import numpy as np
 
 from qiskit import QuantumCircuit, ClassicalRegister, QuantumRegister
-from qiskit.circuit import Qubit, Gate, ControlFlowOp
+from qiskit.circuit import Qubit, Gate, ControlFlowOp, ForLoopOp
 from qiskit.compiler import transpile, assemble
 from qiskit.transpiler import CouplingMap, Layout, PassManager, TranspilerError
 from qiskit.circuit.library import U2Gate, U3Gate, QuantumVolume, CXGate, CZGate, XGate
@@ -445,6 +445,203 @@ def get_translation_stage_plugin(self):
         self.assertIn("PadDynamicalDecoupling", self.passes)
         self.assertIn("RemoveResetInZeroState", self.passes)
 
+    def test_level1_runs_vf2post_layout_when_routing_required(self):
+        """Test that if we run routing as part of sabre layout VF2PostLayout runs."""
+        target = FakeLagosV2()
+        qc = QuantumCircuit(5)
+        qc.h(0)
+        qc.cy(0, 1)
+        qc.cy(0, 2)
+        qc.cy(0, 3)
+        qc.cy(0, 4)
+        qc.measure_all()
+        _ = transpile(qc, target, optimization_level=1, callback=self.callback)
+        # Expected call path for layout and routing is:
+        # 1. TrivialLayout (no perfect match)
+        # 2. VF2Layout (no perfect match)
+        # 3. SabreLayout (heuristic layout and also runs routing)
+        # 4. VF2PostLayout (applies a better layout)
+        self.assertIn("TrivialLayout", self.passes)
+        self.assertIn("VF2Layout", self.passes)
+        self.assertIn("SabreLayout", self.passes)
+        self.assertIn("VF2PostLayout", self.passes)
+        #  Assert we don't run standalone sabre swap
+        self.assertNotIn("SabreSwap", self.passes)
+
+    def test_level1_runs_vf2post_layout_when_routing_method_set_and_required(self):
+        """Test that if we run routing as part of sabre layout VF2PostLayout runs."""
+        target = FakeLagosV2()
+        qc = QuantumCircuit(5)
+        qc.h(0)
+        qc.cy(0, 1)
+        qc.cy(0, 2)
+        qc.cy(0, 3)
+        qc.cy(0, 4)
+        qc.measure_all()
+        _ = transpile(
+            qc, target, optimization_level=1, routing_method="stochastic", callback=self.callback
+        )
+        # Expected call path for layout and routing is:
+        # 1. TrivialLayout (no perfect match)
+        # 2. VF2Layout (no perfect match)
+        # 3. SabreLayout (heuristic layout and also runs routing)
+        # 4. VF2PostLayout (applies a better layout)
+        self.assertIn("TrivialLayout", self.passes)
+        self.assertIn("VF2Layout", self.passes)
+        self.assertIn("SabreLayout", self.passes)
+        self.assertIn("VF2PostLayout", self.passes)
+        self.assertIn("StochasticSwap", self.passes)
+
+    def test_level1_not_runs_vf2post_layout_when_layout_method_set(self):
+        """Test that if we don't run VF2PostLayout with custom layout_method."""
+        target = FakeLagosV2()
+        qc = QuantumCircuit(5)
+        qc.h(0)
+        qc.cy(0, 1)
+        qc.cy(0, 2)
+        qc.cy(0, 3)
+        qc.cy(0, 4)
+        qc.measure_all()
+        _ = transpile(
+            qc, target, optimization_level=1, layout_method="dense", callback=self.callback
+        )
+        # Expected call path for layout and routing is:
+        # 1. TrivialLayout (no perfect match)
+        # 2. VF2Layout (no perfect match)
+        # 3. SabreLayout (heuristic layout and also runs routing)
+        # 4. VF2PostLayout (applies a better layout)
+        self.assertNotIn("TrivialLayout", self.passes)
+        self.assertNotIn("VF2Layout", self.passes)
+        self.assertNotIn("SabreLayout", self.passes)
+        self.assertNotIn("VF2PostLayout", self.passes)
+        self.assertIn("DenseLayout", self.passes)
+        self.assertIn("SabreSwap", self.passes)
+
+    def test_level1_not_run_vf2post_layout_when_trivial_is_perfect(self):
+        """Test that if we find a trivial perfect layout we don't run vf2post."""
+        target = FakeLagosV2()
+        qc = QuantumCircuit(2)
+        qc.h(0)
+        qc.cx(0, 1)
+        qc.measure_all()
+        _ = transpile(qc, target, optimization_level=1, callback=self.callback)
+        self.assertIn("TrivialLayout", self.passes)
+        self.assertNotIn("VF2Layout", self.passes)
+        self.assertNotIn("SabreLayout", self.passes)
+        self.assertNotIn("VF2PostLayout", self.passes)
+        #  Assert we don't run standalone sabre swap
+        self.assertNotIn("SabreSwap", self.passes)
+
+    def test_level1_not_run_vf2post_layout_when_vf2layout_is_perfect(self):
+        """Test that if we find a vf2 perfect layout we don't run vf2post."""
+        target = FakeLagosV2()
+        qc = QuantumCircuit(4)
+        qc.h(0)
+        qc.cx(0, 1)
+        qc.cx(0, 2)
+        qc.cx(0, 3)
+        qc.measure_all()
+        _ = transpile(qc, target, optimization_level=1, callback=self.callback)
+        self.assertIn("TrivialLayout", self.passes)
+        self.assertIn("VF2Layout", self.passes)
+        self.assertNotIn("SabreLayout", self.passes)
+        self.assertNotIn("VF2PostLayout", self.passes)
+        #  Assert we don't run standalone sabre swap
+        self.assertNotIn("SabreSwap", self.passes)
+
+    def test_level1_runs_vf2post_layout_when_routing_required_control_flow(self):
+        """Test that if we run routing as part of sabre layout VF2PostLayout runs."""
+        target = FakeLagosV2()
+        _target = target.target
+        target._target.add_instruction(ForLoopOp, name="for_loop")
+        qc = QuantumCircuit(5)
+        qc.h(0)
+        qc.cy(0, 1)
+        qc.cy(0, 2)
+        qc.cy(0, 3)
+        qc.cy(0, 4)
+        with qc.for_loop((1,)):
+            qc.cx(0, 1)
+        qc.measure_all()
+        _ = transpile(qc, target, optimization_level=1, callback=self.callback)
+        # Expected call path for layout and routing is:
+        # 1. TrivialLayout (no perfect match)
+        # 2. VF2Layout (no perfect match)
+        # 3. SabreLayout (heuristic layout and also runs routing)
+        # 4. VF2PostLayout (applies a better layout)
+        self.assertIn("TrivialLayout", self.passes)
+        self.assertIn("VF2Layout", self.passes)
+        self.assertIn("DenseLayout", self.passes)
+        self.assertIn("StochasticSwap", self.passes)
+        self.assertIn("VF2PostLayout", self.passes)
+
+    def test_level1_not_runs_vf2post_layout_when_layout_method_set_control_flow(self):
+        """Test that if we don't run VF2PostLayout with custom layout_method."""
+        target = FakeLagosV2()
+        _target = target.target
+        target._target.add_instruction(ForLoopOp, name="for_loop")
+        qc = QuantumCircuit(5)
+        qc.h(0)
+        qc.cy(0, 1)
+        qc.cy(0, 2)
+        qc.cy(0, 3)
+        qc.cy(0, 4)
+        with qc.for_loop((1,)):
+            qc.cx(0, 1)
+        qc.measure_all()
+        _ = transpile(
+            qc, target, optimization_level=1, layout_method="dense", callback=self.callback
+        )
+        # Expected call path for layout and routing is:
+        # 1. TrivialLayout (no perfect match)
+        # 2. VF2Layout (no perfect match)
+        # 3. SabreLayout (heuristic layout and also runs routing)
+        # 4. VF2PostLayout (applies a better layout)
+        self.assertNotIn("TrivialLayout", self.passes)
+        self.assertNotIn("VF2Layout", self.passes)
+        self.assertNotIn("SabreLayout", self.passes)
+        self.assertNotIn("VF2PostLayout", self.passes)
+        self.assertIn("DenseLayout", self.passes)
+        self.assertIn("StochasticSwap", self.passes)
+
+    def test_level1_not_run_vf2post_layout_when_trivial_is_perfect_control_flow(self):
+        """Test that if we find a trivial perfect layout we don't run vf2post."""
+        target = FakeLagosV2()
+        _target = target.target
+        target._target.add_instruction(ForLoopOp, name="for_loop")
+        qc = QuantumCircuit(2)
+        qc.h(0)
+        qc.cx(0, 1)
+        with qc.for_loop((1,)):
+            qc.cx(0, 1)
+        qc.measure_all()
+        _ = transpile(qc, target, optimization_level=1, callback=self.callback)
+        self.assertIn("TrivialLayout", self.passes)
+        self.assertNotIn("VF2Layout", self.passes)
+        self.assertNotIn("DenseLayout", self.passes)
+        self.assertNotIn("StochasticSwap", self.passes)
+        self.assertNotIn("VF2PostLayout", self.passes)
+
+    def test_level1_not_run_vf2post_layout_when_vf2layout_is_perfect_control_flow(self):
+        """Test that if we find a vf2 perfect layout we don't run vf2post."""
+        target = FakeLagosV2()
+        _target = target.target
+        target._target.add_instruction(ForLoopOp, name="for_loop")
+        qc = QuantumCircuit(4)
+        qc.h(0)
+        qc.cx(0, 1)
+        qc.cx(0, 2)
+        qc.cx(0, 3)
+        with qc.for_loop((1,)):
+            qc.cx(0, 1)
+        qc.measure_all()
+        _ = transpile(qc, target, optimization_level=1, callback=self.callback)
+        self.assertIn("TrivialLayout", self.passes)
+        self.assertIn("VF2Layout", self.passes)
+        self.assertNotIn("DenseLayout", self.passes)
+        self.assertNotIn("VF2PostLayout", self.passes)
+        self.assertNotIn("StochasticSwap", self.passes)
+
 
 @ddt
 class TestInitialLayouts(QiskitTestCase):
@@ -717,18 +914,18 @@ def test_layout_tokyo_fully_connected_cx(self, level):
 
         sabre_layout = {
             0: ancilla[0],
-            1: ancilla[1],
-            2: ancilla[2],
-            3: ancilla[3],
-            4: ancilla[4],
+            1: qr[4],
+            2: ancilla[1],
+            3: ancilla[2],
+            4: ancilla[3],
             5: qr[1],
-            6: qr[2],
-            7: ancilla[5],
-            8: ancilla[6],
-            9: ancilla[7],
-            10: qr[3],
-            11: qr[0],
-            12: qr[4],
+            6: qr[0],
+            7: ancilla[4],
+            8: ancilla[5],
+            9: ancilla[6],
+            10: qr[2],
+            11: qr[3],
+            12: ancilla[7],
             13: ancilla[8],
             14: ancilla[9],
             15: ancilla[10],

test/python/visualization/references/pass_manager_standard.dot

@@ -55,35 +55,37 @@ labeljust=l;
 16 [color=red, fontname=helvetica, label=CheckMap, shape=rectangle];
 17 [color=black, fontname=helvetica, fontsize=10, label=coupling_map, shape=ellipse, style=solid];
 17 -> 16;
+18 [color=black, fontname=helvetica, fontsize=10, label=property_set_field, shape=ellipse, style=dashed];
+18 -> 16;
 12 -> 16;
 }
 
-subgraph cluster_18 {
+subgraph cluster_19 {
 fontname=helvetica;
 label="[6] do_while";
 labeljust=l;
-19 [color=blue, fontname=helvetica, label=BarrierBeforeFinalMeasurements, shape=rectangle];
-16 -> 19;
+20 [color=blue, fontname=helvetica, label=BarrierBeforeFinalMeasurements, shape=rectangle];
+16 -> 20;
 }
 
-subgraph cluster_20 {
+subgraph cluster_21 {
 fontname=helvetica;
 label="[7] ";
 labeljust=l;
-21 [color=blue, fontname=helvetica, label=GateDirection, shape=rectangle];
-22 [color=black, fontname=helvetica, fontsize=10, label=coupling_map, shape=ellipse, style=solid];
-22 -> 21;
-23 [color=black, fontname=helvetica, fontsize=10, label=target, shape=ellipse, style=dashed];
-23 -> 21;
-19 -> 21;
+22 [color=blue, fontname=helvetica, label=GateDirection, shape=rectangle];
+23 [color=black, fontname=helvetica, fontsize=10, label=coupling_map, shape=ellipse, style=solid];
+23 -> 22;
+24 [color=black, fontname=helvetica, fontsize=10, label=target, shape=ellipse, style=dashed];
+24 -> 22;
+20 -> 22;
 }
 
-subgraph cluster_24 {
+subgraph cluster_25 {
 fontname=helvetica;
 label="[8] ";
 labeljust=l;
-25 [color=blue, fontname=helvetica, label=RemoveResetInZeroState, shape=rectangle];
-21 -> 25;
+26 [color=blue, fontname=helvetica, label=RemoveResetInZeroState, shape=rectangle];
+22 -> 26;
 }
 
 }