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

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)

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.

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()

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()