diff --git a/qiskit/transpiler/passes/layout/sabre_layout.py b/qiskit/transpiler/passes/layout/sabre_layout.py index 312cd51c73bd..7771578f40f2 100644 --- a/qiskit/transpiler/passes/layout/sabre_layout.py +++ b/qiskit/transpiler/passes/layout/sabre_layout.py @@ -319,8 +319,12 @@ def to_partial_layout(layout): if (prev_final_layout := self.property_set.get("final_layout", None)) is None: self.property_set["final_layout"] = final_layout else: - self.property_set["final_layout"] = final_layout.compose( - prev_final_layout, out_dag.qubits + # The "final layout" can be thought of as a "comes from" permutation that you apply at + # the end of the circuit to invert the routing. So if there's an existing one, what we + # apply at the end of the circuit needs to set the circuit qubits so they "come from" + # the previous one, then those "come from" the one we've just added. + self.property_set["final_layout"] = prev_final_layout.compose( + final_layout, out_dag.qubits ) return out_dag diff --git a/qiskit/transpiler/passes/routing/basic_swap.py b/qiskit/transpiler/passes/routing/basic_swap.py index e1245051d42b..70b20eba0a87 100644 --- a/qiskit/transpiler/passes/routing/basic_swap.py +++ b/qiskit/transpiler/passes/routing/basic_swap.py @@ -117,8 +117,12 @@ def run(self, dag): if self.property_set["final_layout"] is None: self.property_set["final_layout"] = current_layout else: - self.property_set["final_layout"] = current_layout.compose( - self.property_set["final_layout"], dag.qubits + # The "final layout" can be thought of as a "comes from" permutation that you apply at + # the end of the circuit to invert the routing. So if there's an existing one, what we + # apply at the end of the circuit needs to set the circuit qubits so they "come from" + # the previous one, then those "come from" the one we've just added. + self.property_set["final_layout"] = self.property_set["final_layout"].compose( + current_layout, dag.qubits ) return new_dag @@ -160,7 +164,11 @@ def _fake_run(self, dag): if self.property_set["final_layout"] is None: self.property_set["final_layout"] = current_layout else: - self.property_set["final_layout"] = current_layout.compose( - self.property_set["final_layout"], dag.qubits + # The "final layout" can be thought of as a "comes from" permutation that you apply at + # the end of the circuit to invert the routing. So if there's an existing one, what we + # apply at the end of the circuit needs to set the circuit qubits so they "come from" + # the previous one, then those "come from" the one we've just added. + self.property_set["final_layout"] = self.property_set["final_layout"].compose( + current_layout, dag.qubits ) return dag diff --git a/qiskit/transpiler/passes/routing/lookahead_swap.py b/qiskit/transpiler/passes/routing/lookahead_swap.py index 4c6462c04a87..98d09874eb14 100644 --- a/qiskit/transpiler/passes/routing/lookahead_swap.py +++ b/qiskit/transpiler/passes/routing/lookahead_swap.py @@ -172,8 +172,12 @@ def run(self, dag): if self.property_set["final_layout"] is None: self.property_set["final_layout"] = current_state.layout else: - self.property_set["final_layout"] = current_state.layout.compose( - self.property_set["final_layout"], dag.qubits + # The "final layout" can be thought of as a "comes from" permutation that you apply at + # the end of the circuit to invert the routing. So if there's an existing one, what we + # apply at the end of the circuit needs to set the circuit qubits so they "come from" + # the previous one, then those "come from" the one we've just added. + self.property_set["final_layout"] = self.property_set["final_layout"].compose( + current_state.layout, dag.qubits ) if self.fake_run: diff --git a/qiskit/transpiler/passes/routing/sabre_swap.py b/qiskit/transpiler/passes/routing/sabre_swap.py index bbc7a16c5da9..2868bac197b9 100644 --- a/qiskit/transpiler/passes/routing/sabre_swap.py +++ b/qiskit/transpiler/passes/routing/sabre_swap.py @@ -245,6 +245,10 @@ def run(self, dag): self.property_set["final_layout"] = ( layout if (prev := self.property_set["final_layout"]) is None - else layout.compose(prev, dag.qubits) + # The "final layout" can be thought of as a "comes from" permutation that you apply at + # the end of the circuit to invert the routing. So if there's an existing one, what we + # apply at the end of the circuit needs to set the circuit qubits so they "come from" + # the previous one, then those "come from" the one we've just added. + else prev.compose(layout, dag.qubits) ) return dag diff --git a/releasenotes/notes/fix-final-layout-8b027fb01e831ef9.yaml b/releasenotes/notes/fix-final-layout-8b027fb01e831ef9.yaml new file mode 100644 index 000000000000..c8d5311905d2 --- /dev/null +++ b/releasenotes/notes/fix-final-layout-8b027fb01e831ef9.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Built-in transpiler passes that set the ``final_layout`` property will now correctly handle + updating this field if it was already set. This can be observed as the method + :attr:`.TranspileLayout.routing_permutation` now returning a correct permutation after running + more than one pass that sets ``final_layout``. + + This did not affect any normal calls to :func:`.transpile` or :func:`.generate_preset_pass_manager` + using Qiskit's built-in plugins; no pipeline constructed in this form would attempt to set + ``final_layout`` more than once. diff --git a/test/python/compiler/test_transpiler.py b/test/python/compiler/test_transpiler.py index e5a0f4340d8d..202c78db2858 100644 --- a/test/python/compiler/test_transpiler.py +++ b/test/python/compiler/test_transpiler.py @@ -18,6 +18,7 @@ import math import os import sys +import random from logging import StreamHandler, getLogger from unittest.mock import patch import numpy as np @@ -62,6 +63,7 @@ ECRGate, HGate, IGate, + PermutationGate, PhaseGate, RXGate, RYGate, @@ -85,7 +87,7 @@ from qiskit.providers.options import Options from qiskit.quantum_info import Operator, random_unitary from qiskit.utils import should_run_in_parallel -from qiskit.transpiler import CouplingMap, Layout, PassManager +from qiskit.transpiler import CouplingMap, Layout, PassManager, passes from qiskit.transpiler.exceptions import TranspilerError, CircuitTooWideForTarget from qiskit.transpiler.passes import BarrierBeforeFinalMeasurements, GateDirection, VF2PostLayout @@ -2729,6 +2731,50 @@ def callback(**kwargs): self.assertTrue(vf2_post_layout_called) self.assertEqual([2, 1, 0], _get_index_layout(tqc, qubits)) + @data("sabre", "lookahead", "basic") + def test_final_layout_combined_correctly(self, routing): + """Test that multiple `final_layout`s are combined correctly.""" + generators = { + "sabre": lambda cmap, seed: passes.SabreSwap(cmap, seed=seed, trials=1), + "lookahead": lambda cmap, _seed: passes.LookaheadSwap(cmap), + "basic": lambda cmap, seed: passes.BasicSwap(cmap), + } + make_routing_pass = generators[routing] + + def random_line(num_qubits, rng): + line = list(range(num_qubits)) + rng.shuffle(line) + out = CouplingMap([[a, b] for a, b in zip(line[:-1], line[1:])]) + out.make_symmetric() + return out + + rng = random.Random(0) + num_qubits = 5 + + # This is just loads of stars, so routing has to work for it. + qc = QuantumCircuit(num_qubits) + for i in range(num_qubits): + for j in range(num_qubits): + if i == j: + continue + qc.cx(i, j) + # ... and the mirror, so the circuit implements the identity. + qc.barrier() + qc.compose(qc.inverse(), qc.qubits, inplace=True) + + # Routing needs a layout set, and we don't want qubit relabelling to mess with our test. + pm = PassManager([passes.SetLayout(list(range(num_qubits))), passes.ApplyLayout()]) + # Now we route the circuit to several different coupling maps in a row, so we generate lots + # of routing permutations, and each pass needs to combine them. + pm += PassManager([make_routing_pass(random_line(num_qubits, rng), i) for i in range(5)]) + out = pm.run(qc) + + # The invariant of `routing_permutation` is that you're supposed to be able to append it as + # a permutation and it will "undo" the effects. We already know our circuit implements the + # identity. + out.append(PermutationGate(out.layout.routing_permutation()), out.qubits) + self.assertEqual(Operator(out), Operator(np.eye(2**num_qubits))) + @data(0, 1, 2, 3) def test_annotations_survive(self, optimization_level): """Test that custom annotations survive a full transpile."""