From 26886a1b2926a474ea06aa3f9ce9e11e6ce28020 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Fri, 10 Feb 2023 17:52:59 +0000 Subject: [PATCH] Make `BarrierBeforeFinalMeasurements` deterministic (#9568) * Make `BarrierBeforeFinalMeasurements` deterministic The barrier-merging step at the end directly converted its inner (unordered) sets of bits into (ordered) lists, which is an operation dependent on `PYTHONHASHSEED`. This can have an impact on subsequent topological iteration orders, if there are any barriers that are not full-width in the circuit, which can in turn cause later stochastic transpiler passes to have different outputs for fixed random seeds, depending on `PYTHONHASHSEED`. * Add direct test of MergeAdjacentBarriers --- .../passes/utils/merge_adjacent_barriers.py | 5 +++- ...e-final-measurements-04e817d995794067.yaml | 9 +++++++ .../transpiler/test_adjacent_barriers.py | 23 ++++++++++++++++++ .../test_barrier_before_final_measurements.py | 24 +++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/deterministic-barrier-before-final-measurements-04e817d995794067.yaml diff --git a/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py b/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py index 129fce9469db..f77870047c9d 100644 --- a/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py +++ b/qiskit/transpiler/passes/utils/merge_adjacent_barriers.py @@ -57,6 +57,7 @@ class MergeAdjacentBarriers(TransformationPass): def run(self, dag): """Run the MergeAdjacentBarriers pass on `dag`.""" + indices = {qubit: index for index, qubit in enumerate(dag.qubits)} # sorted to so that they are in the order they appear in the DAG # so ancestors/descendants makes sense @@ -77,7 +78,9 @@ def run(self, dag): if node in node_to_barrier_qubits: qubits = node_to_barrier_qubits[node] # qubits are stored as a set, need to convert to a list - new_dag.apply_operation_back(Barrier(len(qubits)), qargs=list(qubits)) + new_dag.apply_operation_back( + Barrier(len(qubits)), qargs=sorted(qubits, key=indices.get) + ) else: # copy the condition over too new_dag.apply_operation_back(node.op, qargs=node.qargs, cargs=node.cargs) diff --git a/releasenotes/notes/deterministic-barrier-before-final-measurements-04e817d995794067.yaml b/releasenotes/notes/deterministic-barrier-before-final-measurements-04e817d995794067.yaml new file mode 100644 index 000000000000..eda25b1e4763 --- /dev/null +++ b/releasenotes/notes/deterministic-barrier-before-final-measurements-04e817d995794067.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + The :class:`.BarrierBeforeFinalMeasurements` and :class:`.MergeAdjacentBarriers` transpiler + passes previously had a non-deterministic order of their emitted :class:`.Barrier` instructions. + This did not change the semantics of circuits but could, in limited cases where there were + non-full-width barriers, cause later stochastic transpiler passes to see a different topological + ordering of the circuit and consequently have different outputs for fixed seeds. The passes + have been made deterministic to avoid this. diff --git a/test/python/transpiler/test_adjacent_barriers.py b/test/python/transpiler/test_adjacent_barriers.py index c6cb13372ca7..e979a6a25bef 100644 --- a/test/python/transpiler/test_adjacent_barriers.py +++ b/test/python/transpiler/test_adjacent_barriers.py @@ -12,6 +12,7 @@ """Test the MergeAdjacentBarriers pass""" +import random import unittest from qiskit.transpiler.passes import MergeAdjacentBarriers from qiskit.converters import circuit_to_dag @@ -271,6 +272,28 @@ def test_barriers_with_blocking_obstacle_twoQ(self): self.assertEqual(result, circuit_to_dag(expected)) + def test_output_deterministic(self): + """Test that the output barriers have a deterministic ordering (independent of + PYTHONHASHSEED). This is important to guarantee that any subsequent topological iterations + through the circuit are also deterministic; it's in general not possible for all transpiler + passes to produce identical outputs across all valid topological orderings, especially if + those passes have some stochastic element.""" + order = list(range(20)) + random.Random(2023_02_10).shuffle(order) + circuit = QuantumCircuit(20) + circuit.barrier([5, 2, 3]) + circuit.barrier([7, 11, 14, 2, 4]) + circuit.barrier(order) + + # All the barriers should get merged together. + expected = QuantumCircuit(20) + expected.barrier(range(20)) + + output = MergeAdjacentBarriers()(circuit) + self.assertEqual(expected, output) + # This assertion is that the ordering of the arguments in the barrier is fixed. + self.assertEqual(list(output.data[0].qubits), list(output.qubits)) + if __name__ == "__main__": unittest.main() diff --git a/test/python/transpiler/test_barrier_before_final_measurements.py b/test/python/transpiler/test_barrier_before_final_measurements.py index 6edc7b3fa959..4b879e0e290a 100644 --- a/test/python/transpiler/test_barrier_before_final_measurements.py +++ b/test/python/transpiler/test_barrier_before_final_measurements.py @@ -12,6 +12,7 @@ """Test the BarrierBeforeFinalMeasurements pass""" +import random import unittest from qiskit.transpiler.passes import BarrierBeforeFinalMeasurements from qiskit.converters import circuit_to_dag @@ -392,6 +393,29 @@ def test_conditioned_on_single_bit(self): pass_ = BarrierBeforeFinalMeasurements() self.assertEqual(expected, pass_(circuit)) + def test_output_deterministic(self): + """Test that the output barriers have a deterministic ordering (independent of + PYTHONHASHSEED). This is important to guarantee that any subsequent topological iterations + through the circuit are also deterministic; it's in general not possible for all transpiler + passes to produce identical outputs across all valid topological orderings, especially if + those passes have some stochastic element.""" + measure_order = list(range(20)) + random.Random(2023_02_10).shuffle(measure_order) + circuit = QuantumCircuit(20, 20) + circuit.barrier([5, 2, 3]) + circuit.barrier([7, 11, 14, 2, 4]) + circuit.measure(measure_order, measure_order) + + # All the barriers should get merged together. + expected = QuantumCircuit(20, 20) + expected.barrier(range(20)) + expected.measure(measure_order, measure_order) + + output = BarrierBeforeFinalMeasurements()(circuit) + self.assertEqual(expected, output) + # This assertion is that the ordering of the arguments in the barrier is fixed. + self.assertEqual(list(output.data[0].qubits), list(output.qubits)) + if __name__ == "__main__": unittest.main()