diff --git a/qiskit/transpiler/passes/routing/sabre_swap.py b/qiskit/transpiler/passes/routing/sabre_swap.py index 3690c763f734..5e44b29e5d13 100644 --- a/qiskit/transpiler/passes/routing/sabre_swap.py +++ b/qiskit/transpiler/passes/routing/sabre_swap.py @@ -18,7 +18,6 @@ import numpy as np from qiskit.circuit.library.standard_gates import SwapGate -from qiskit.circuit.quantumregister import Qubit from qiskit.transpiler.basepasses import TransformationPass from qiskit.transpiler.exceptions import TranspilerError from qiskit.transpiler.layout import Layout @@ -276,15 +275,18 @@ def _reset_qubits_decay(self): self.qubits_decay = {k: 1 for k in self.qubits_decay.keys()} def _successors(self, node, dag): - for _, successor, edge_data in dag.edges(node): - if not isinstance(successor, DAGOpNode): - continue - if isinstance(edge_data, Qubit): + """Return an iterable of the successors along each wire from the given node. + + This yields the same successor multiple times if there are parallel wires (e.g. two adjacent + operations that have one clbit and qubit in common), which is important in the swapping + algorithm for detecting if each wire has been accounted for.""" + for _, successor, _ in dag.edges(node): + if isinstance(successor, DAGOpNode): yield successor def _is_resolved(self, node): """Return True if all of a node's predecessors in dag are applied.""" - return self.applied_predecessors[node] == len(node.qargs) + return self.applied_predecessors[node] == len(node.qargs) + len(node.cargs) def _obtain_extended_set(self, dag, front_layer): """Populate extended_set by looking ahead a fixed number of gates. diff --git a/releasenotes/notes/fix-sabreswap-clbits-428eb5f3a46063da.yaml b/releasenotes/notes/fix-sabreswap-clbits-428eb5f3a46063da.yaml new file mode 100644 index 000000000000..0401e5a36f74 --- /dev/null +++ b/releasenotes/notes/fix-sabreswap-clbits-428eb5f3a46063da.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixed :class:`.SabreSwap`, and by extension :func:`.transpile` with + ``optimization_level=3``, occasionally re-ordering measurements invalidly. + Previously, if two measurements wrote to the same classical bit, + :class:`.SabreSwap` could (depending on the coupling map) re-order them to + produce a non-equivalent circuit. This behaviour was stochastic, so may + not have appeared reliably. diff --git a/test/python/transpiler/test_sabre_swap.py b/test/python/transpiler/test_sabre_swap.py index a19f574a305d..509225f65e75 100644 --- a/test/python/transpiler/test_sabre_swap.py +++ b/test/python/transpiler/test_sabre_swap.py @@ -13,6 +13,7 @@ """Test the Sabre Swap pass""" import unittest +from qiskit.circuit.library import CCXGate, HGate, Measure from qiskit.transpiler.passes import SabreSwap from qiskit.transpiler import CouplingMap, PassManager from qiskit import QuantumRegister, QuantumCircuit @@ -95,6 +96,34 @@ def test_do_not_change_cm(self): self.assertEqual(set(cm_edges), set(coupling.get_edges())) + def test_do_not_reorder_measurements(self): + """Test that SabreSwap doesn't reorder measurements to the same classical bit. + + With the particular coupling map used in this test and the 3q ccx gate, the routing would + invariably the measurements if the classical successors are not accurately tracked. + Regression test of gh-7950.""" + coupling = CouplingMap([(0, 2), (2, 0), (1, 2), (2, 1)]) + qc = QuantumCircuit(3, 1) + qc.compose(CCXGate().definition, [0, 1, 2], []) # Unroll CCX to 2q operations. + qc.h(0) + qc.barrier() + qc.measure(0, 0) # This measure is 50/50 between the Z states. + qc.measure(1, 0) # This measure always overwrites with 0. + passmanager = PassManager(SabreSwap(coupling)) + transpiled = passmanager.run(qc) + + last_h, h_qubits, _ = transpiled.data[-4] + self.assertIsInstance(last_h, HGate) + first_measure, first_measure_qubits, _ = transpiled.data[-2] + second_measure, second_measure_qubits, _ = transpiled.data[-1] + self.assertIsInstance(first_measure, Measure) + self.assertIsInstance(second_measure, Measure) + # Assert that the first measure is on the same qubit that the HGate was applied to, and the + # second measurement is on a different qubit (though we don't care which exactly - that + # depends a little on the randomisation of the pass). + self.assertEqual(h_qubits, first_measure_qubits) + self.assertNotEqual(h_qubits, second_measure_qubits) + if __name__ == "__main__": unittest.main()