From 9c58064b6a6edab79c96123a310ac0940de62d1c Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 30 Jan 2024 04:03:05 -0500 Subject: [PATCH] Fix split barriers leaking during disjoint layout processing (#11655) * Fix split barriers leaking during disjoint layout processing This commit fixes an issue in the disjoint layout processing caused by a shared reference from the input dag to a layout pass to the internal transformations the layout processing performs. As part of the disjoint layout processing the input dag needs to be split into smaller dags for each connected component in the circuit. To enable this any barriers present in the circuit need to be split up prior to analyzing the connected components in the dag. A multiqubit barrier doesn't represent a real connectivity requirement so they need to be split prior to analysis to ensure they don't factor into the connected component computation. To faciliate not losing the semantic meaning of the barrier for the layout analysis of each connected component, the barrier split is done by first taking an n-qubit barrier and converting it into n parallel 1q barriers with a uuid label applied to it. Then after we split the circuit into the separate connected components the uuid is used to identify any components of the same barrier and combine them together. There were two issues with this approach the first is that the splitting of the barriers was done on the input dag without copying first. This causes the input dag to be modified and because in most cases it's a shared instance which would leak out the barrier splitting if the input dag was returned verbatim from the pass, which in most cases it would be. This issue is fixed by ensuring that we re-combine any split barriers after we've split the dag into it's connected components. The second issue is the uuid label assignment would overwrite any existing labels on barriers. While setting labels on barriers is uncommon for users to do (but still completely valid) this is causing a bigger issues since #10323 because the transpiler is assigning labels to barriers it creates internally so they can be removed before the circuit is returned. This is also fixed in this commit by appending a uuid to the existing label instead of overwriting it, so we're able to restore the original label when we recombine barriers. Fixes #11649 * Always use string uuid * Add release note --- .../passes/layout/disjoint_utils.py | 22 ++++++++++++++----- ...eaking-split-barrier-7c143d6b13b96ced.yaml | 12 ++++++++++ test/python/compiler/test_transpiler.py | 13 +++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/fix-leaking-split-barrier-7c143d6b13b96ced.yaml diff --git a/qiskit/transpiler/passes/layout/disjoint_utils.py b/qiskit/transpiler/passes/layout/disjoint_utils.py index ffbb5076c1ef..42f665c473a1 100644 --- a/qiskit/transpiler/passes/layout/disjoint_utils.py +++ b/qiskit/transpiler/passes/layout/disjoint_utils.py @@ -107,7 +107,10 @@ def split_barriers(dag: DAGCircuit): num_qubits = len(node.qargs) if num_qubits == 1: continue - barrier_uuid = uuid.uuid4() + if node.op.label: + barrier_uuid = f"{node.op.label}_uuid={uuid.uuid4()}" + else: + barrier_uuid = f"_none_uuid={uuid.uuid4()}" split_dag = DAGCircuit() split_dag.add_qubits([Qubit() for _ in range(num_qubits)]) for i in range(num_qubits): @@ -124,10 +127,13 @@ def combine_barriers(dag: DAGCircuit, retain_uuid: bool = True): qubit_indices = {bit: index for index, bit in enumerate(dag.qubits)} uuid_map: dict[uuid.UUID, DAGOpNode] = {} for node in dag.op_nodes(Barrier): - if isinstance(node.op.label, uuid.UUID): - barrier_uuid = node.op.label + if node.op.label: + if "_uuid=" in node.op.label: + barrier_uuid = node.op.label + else: + continue if barrier_uuid in uuid_map: - other_node = uuid_map[node.op.label] + other_node = uuid_map[barrier_uuid] num_qubits = len(other_node.qargs) + len(node.qargs) new_op = Barrier(num_qubits, label=barrier_uuid) new_node = dag.replace_block_with_op([node, other_node], new_op, qubit_indices) @@ -136,8 +142,11 @@ def combine_barriers(dag: DAGCircuit, retain_uuid: bool = True): uuid_map[barrier_uuid] = node if not retain_uuid: for node in dag.op_nodes(Barrier): - if isinstance(node.op.label, uuid.UUID): + if isinstance(node.op.label, str) and node.op.label.startswith("_none_uuid="): node.op.label = None + elif isinstance(node.op.label, str) and "_uuid=" in node.op.label: + original_label = "_uuid=".join(node.op.label.split("_uuid=")[:-1]) + node.op.label = original_label def require_layout_isolated_to_component( @@ -202,4 +211,7 @@ def separate_dag(dag: DAGCircuit) -> List[DAGCircuit]: new_dag.remove_clbits(*idle_clbits) combine_barriers(new_dag) decomposed_dags.append(new_dag) + # Reverse split barriers on input dag to avoid leaking out internal transformations as + # part of splitting + combine_barriers(dag, retain_uuid=False) return decomposed_dags diff --git a/releasenotes/notes/fix-leaking-split-barrier-7c143d6b13b96ced.yaml b/releasenotes/notes/fix-leaking-split-barrier-7c143d6b13b96ced.yaml new file mode 100644 index 000000000000..5cd8f19dc150 --- /dev/null +++ b/releasenotes/notes/fix-leaking-split-barrier-7c143d6b13b96ced.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Fixed an issue when using :func:`.transpile` or running a preset pass + manager (such as generated by :func:`.generate_preset_pass_manager`) when + targetting a backend that has disjoint connectivity adding extra barriers + to the output :class:`.QuantumCircuit`. In some cases several + single qubit :class:`.Barrier` directives would be included in the output + circuit right before any final measurements in the circuit. This was + internal state generated by the internal processing for disjoint + connectivity that was incorrectly being added into the output circuit. + Fixed `#11649 `__ diff --git a/test/python/compiler/test_transpiler.py b/test/python/compiler/test_transpiler.py index 1aa34edbb50c..181687fe6901 100644 --- a/test/python/compiler/test_transpiler.py +++ b/test/python/compiler/test_transpiler.py @@ -3381,6 +3381,19 @@ def test_transpile_target_with_qubits_without_ops_circuit_too_large_disconnected with self.assertRaises(TranspilerError): transpile(qc, target=target, optimization_level=opt_level) + @data(0, 1, 2, 3) + def test_barrier_no_leak_disjoint_connectivity(self, opt_level): + """Test that we don't leak an internal labelled barrier from disjoint layout processing.""" + cmap = CouplingMap([(0, 1), (1, 2), (3, 4)]) + qc = QuantumCircuit(cmap.size(), cmap.size()) + qc.cx(0, 1) + qc.cx(1, 2) + qc.cx(2, 0) + qc.cx(3, 4) + qc.measure(qc.qubits, qc.clbits) + out = transpile(qc, coupling_map=cmap, optimization_level=opt_level) + self.assertNotIn("barrier", out.count_ops()) + @data(0, 1, 2, 3) def test_transpile_does_not_affect_backend_coupling(self, opt_level): """Test that transpiliation of a circuit does not mutate the `CouplingMap` stored by a V2