From 6071f054f8f613b35f1bdcf4f73caaed0e2eb3a4 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 11 Apr 2023 09:00:58 -0400 Subject: [PATCH 1/2] 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 | 24 +- .../transpiler/preset_passmanagers/common.py | 6 +- .../vf2postlayout-fix-16bb54d9bdf3aaf6.yaml | 23 ++ .../transpiler/test_preset_passmanagers.py | 221 +++++++++++++++++- .../references/pass_manager_standard.dot | 28 +-- .../references/pass_manager_style.dot | 28 +-- 6 files changed, 281 insertions(+), 49 deletions(-) create mode 100644 releasenotes/notes/vf2postlayout-fix-16bb54d9bdf3aaf6.yaml diff --git a/qiskit/transpiler/passes/utils/check_map.py b/qiskit/transpiler/passes/utils/check_map.py index 69bc9707d318..f71e5640f163 100644 --- a/qiskit/transpiler/passes/utils/check_map.py +++ b/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 diff --git a/qiskit/transpiler/preset_passmanagers/common.py b/qiskit/transpiler/preset_passmanagers/common.py index a340aee45d8d..02654016f4d6 100644 --- a/qiskit/transpiler/preset_passmanagers/common.py +++ b/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) diff --git a/releasenotes/notes/vf2postlayout-fix-16bb54d9bdf3aaf6.yaml b/releasenotes/notes/vf2postlayout-fix-16bb54d9bdf3aaf6.yaml new file mode 100644 index 000000000000..9ff099b927f7 --- /dev/null +++ b/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 `__ diff --git a/test/python/transpiler/test_preset_passmanagers.py b/test/python/transpiler/test_preset_passmanagers.py index 019750c76fc6..098bf896a614 100644 --- a/test/python/transpiler/test_preset_passmanagers.py +++ b/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], diff --git a/test/python/visualization/references/pass_manager_standard.dot b/test/python/visualization/references/pass_manager_standard.dot index d4b1d30422d5..642f51b0a24f 100644 --- a/test/python/visualization/references/pass_manager_standard.dot +++ b/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; } } diff --git a/test/python/visualization/references/pass_manager_style.dot b/test/python/visualization/references/pass_manager_style.dot index 5cfd2a95ea56..64eebf64c08e 100644 --- a/test/python/visualization/references/pass_manager_style.dot +++ b/test/python/visualization/references/pass_manager_style.dot @@ -55,35 +55,37 @@ labeljust=l; 16 [color=green, 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=grey, fontname=helvetica, label=RemoveResetInZeroState, shape=rectangle]; -21 -> 25; +26 [color=grey, fontname=helvetica, label=RemoveResetInZeroState, shape=rectangle]; +22 -> 26; } } From 5eda70a2fd2691a2ea1429f1576dc31e6eb9efd0 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 11 Apr 2023 14:01:14 -0400 Subject: [PATCH 2/2] Fix remove out of date comments --- test/python/transpiler/test_preset_passmanagers.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/test/python/transpiler/test_preset_passmanagers.py b/test/python/transpiler/test_preset_passmanagers.py index 098bf896a614..4d5eb4f45049 100644 --- a/test/python/transpiler/test_preset_passmanagers.py +++ b/test/python/transpiler/test_preset_passmanagers.py @@ -505,11 +505,6 @@ def test_level1_not_runs_vf2post_layout_when_layout_method_set(self): _ = 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) @@ -567,7 +562,8 @@ def test_level1_runs_vf2post_layout_when_routing_required_control_flow(self): # 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) + # 3. DenseLayout (heuristic layout) + # 4. StochasticSwap # 4. VF2PostLayout (applies a better layout) self.assertIn("TrivialLayout", self.passes) self.assertIn("VF2Layout", self.passes) @@ -592,11 +588,6 @@ def test_level1_not_runs_vf2post_layout_when_layout_method_set_control_flow(self _ = 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)