Ensure we run VF2PostLayout when needed in optimization level 1

This commit fixes an issue where we were incorrectly not running
VF2PostLayout in the one case where it should be run. Optimization level
1 is more involved than higher optimization levels because for backwards
compatibility it runs multiple different passes to try and find a
perfect layout, first starting with TrivialLayout, then VF2Layout, and
only if those 2 fail to find a perfect match do we run SabreLayout (or
DenseLayout and StochasticSwap if control flow are present). We only
should run VF2PostLayout if we're running the heuristic layout pass.
However, when we integrated routing into SabreLayout the checking for
whether we found a pefect layout in the layout stage stopped working as
expected.

To fix this issue a new flag is added to the CheckMap pass to specify an
alternative property set field to store the results of the check in. The
underlying issue was that the property set field between the earlier
check that we found a perfect layout and the later check whether we
should run a standalone routing pass. To fix this the new flag is used
to spearate the results from the multiple CheckMap calls. and then we
only condition the VF2PostLayout condition on the results of the perfect
layout check and not the later routing check.

Fixes #9936

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;
 }
 
 }