Skip to content

Commit

Permalink
Fix split barriers leaking during disjoint layout processing (#11655)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mtreinish authored Jan 30, 2024
1 parent bcc1307 commit 9c58064
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 5 deletions.
22 changes: 17 additions & 5 deletions qiskit/transpiler/passes/layout/disjoint_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -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
12 changes: 12 additions & 0 deletions releasenotes/notes/fix-leaking-split-barrier-7c143d6b13b96ced.yaml
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/Qiskit/qiskit/issues/11649>`__
13 changes: 13 additions & 0 deletions test/python/compiler/test_transpiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9c58064

Please sign in to comment.