Skip to content

Commit e0237ef

Browse files
Fix reuse of ConsolidateBlocks instances (Qiskit#15258) (Qiskit#15266)
* Fix reuse of `ConsolidateBlocks` instances The `_qubit_map` state added to `ConsolidateBlocks` in Qiskit#15083 (cb103d9) was attempting to get round an awkward internal `PassManager` use to pass the mapping into recursive calls. Unfortunately, the state was not cleared on exit to the pass, so re-use of the pass object would be invalid, such as by calling the `PassManager.run` method on the result of `generate_preset_pass_manager` more than once on circuits with different sizes. Rather than trying to plaster over the poor use of state, this commit instead moves the runtime logic to be in the `PropertySet`, so re-use troubles are limited to single executions. It's still technically possible for the `PropertySet` to become polluted if `ConsolidateBlocks` throws an internal exception that is forcibly suppressed such that the pipeline continues, but this is completely non-standard, would require significant effort to achieve, and most likely there are large numbers of transpiler passes that are not completely exception-safe anyway. * Add test of `PassManager.run` over a list On some platforms this will be multiprocessing, but it doesn't hurt to try it for the platforms that aren't. * Document use of property-set key (cherry picked from commit 4019fd9) Co-authored-by: Jake Lishman <[email protected]>
1 parent 46a7437 commit e0237ef

File tree

4 files changed

+98
-16
lines changed

4 files changed

+98
-16
lines changed

qiskit/transpiler/passes/optimization/consolidate_blocks.py

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,20 @@ class ConsolidateBlocks(TransformationPass):
6262
the same qubits into a Unitary node, to be resynthesized later,
6363
to a potentially more optimal subcircuit.
6464
65+
This pass reads the :class:`.PropertySet` key ``ConsolidateBlocks_qubit_map`` which it uses to
66+
communicate with recursive worker instances of itself for control-flow operations. The key
67+
should never be observable in a user-facing :class:`.PassManager` pipeline (it is only set in
68+
internal :class:`.PassManager` instances), but the pass may return incorrect results or error if
69+
another pass sets this key.
70+
6571
Notes:
6672
This pass assumes that the 'blocks_list' property that it reads is
6773
given such that blocks are in topological order. The blocks are
6874
collected by a previous pass, such as `Collect2qBlocks`.
6975
"""
7076

77+
_QUBIT_MAP_KEY = "ConsolidateBlocks_qubit_map"
78+
7179
def __init__(
7280
self,
7381
kak_basis_gate=None,
@@ -119,9 +127,6 @@ def __init__(
119127
else:
120128
self.decomposer = TwoQubitBasisDecomposer(CXGate())
121129
self.basis_gate_name = "cx"
122-
# Statefully used within the control-flow block recursion to map back to top-level hardware
123-
# qubits.
124-
self._qubit_map = None
125130

126131
def run(self, dag):
127132
"""Run the ConsolidateBlocks pass on `dag`.
@@ -139,9 +144,9 @@ def run(self, dag):
139144
if runs is not None:
140145
runs = [[node._node_id for node in run] for run in runs]
141146

142-
if self._qubit_map is None:
143-
self._qubit_map = list(range(dag.num_qubits()))
144-
147+
qubit_map = self.property_set.get(self._QUBIT_MAP_KEY, None)
148+
if qubit_map is None:
149+
qubit_map = list(range(dag.num_qubits()))
145150
consolidate_blocks(
146151
dag,
147152
self.decomposer._inner_decomposer,
@@ -151,9 +156,9 @@ def run(self, dag):
151156
basis_gates=self.basis_gates,
152157
blocks=blocks,
153158
runs=runs,
154-
qubit_map=self._qubit_map,
159+
qubit_map=qubit_map,
155160
)
156-
dag = self._handle_control_flow_ops(dag)
161+
dag = self._handle_control_flow_ops(dag, qubit_map)
157162

158163
# Clear collected blocks and runs as they are no longer valid after consolidation
159164
if "run_list" in self.property_set:
@@ -163,7 +168,7 @@ def run(self, dag):
163168

164169
return dag
165170

166-
def _handle_control_flow_ops(self, dag):
171+
def _handle_control_flow_ops(self, dag, qubit_map):
167172
"""
168173
This is similar to transpiler/passes/utils/control_flow.py except that the
169174
collect blocks is redone for the control flow blocks.
@@ -176,12 +181,10 @@ def _handle_control_flow_ops(self, dag):
176181
pass_manager.append(self)
177182

178183
for node in dag.control_flow_op_nodes():
179-
old_qubit_map, self._qubit_map = self._qubit_map, [
180-
self._qubit_map[dag.find_bit(q).index] for q in node.qargs
181-
]
182-
try:
183-
new_op = node.op.replace_blocks(pass_manager.run(block) for block in node.op.blocks)
184-
finally:
185-
self._qubit_map = old_qubit_map
184+
inner_qubit_map = [qubit_map[dag.find_bit(q).index] for q in node.qargs]
185+
new_op = node.op.replace_blocks(
186+
pass_manager.run(block, property_set={self._QUBIT_MAP_KEY: inner_qubit_map})
187+
for block in node.op.blocks
188+
)
186189
dag.substitute_node(node, new_op)
187190
return dag
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
fixes:
3+
- |
4+
Fixed re-use of the same :class:`.ConsolidateBlocks` instance on multiple circuits, including
5+
calls to :func:`.transpile` with more than one circuit and no process-based parallelization. A
6+
bug introduced in Qiskit 2.2.2 caused the pass to panic or produce invalid output if the same
7+
instance was re-used on differing circuits.
8+
upgrade:
9+
- |
10+
:class:`.ConsolidateBlocks` now reads a :class:`.PropertySet` key ``ConsolidateBlocks_qubit_map``
11+
on entry. This key and its value are not public and should not be read or written to by other
12+
passes.

test/python/compiler/test_transpiler.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2333,6 +2333,55 @@ def test_single_qubit_circuit_deterministic_output(self, optimization_level):
23332333
for i in range(10):
23342334
self.assertEqual(isa_circs[0], isa_circs[i])
23352335

2336+
@data(0, 1, 2, 3)
2337+
def test_reuse_on_differing_circuits(self, optimization_level):
2338+
"""Test that re-using the same `PassManager` instance on two different circuits is the same
2339+
as using new instances of the `PassManager`."""
2340+
2341+
def make_pm():
2342+
num_qubits = 12
2343+
target = Target()
2344+
target.add_instruction(
2345+
SXGate(),
2346+
{(i,): InstructionProperties(error=1e-5 * (i + 1)) for i in range(num_qubits)},
2347+
)
2348+
target.add_instruction(
2349+
RZGate(Parameter("a")), {(i,): InstructionProperties() for i in range(num_qubits)}
2350+
)
2351+
target.add_instruction(
2352+
Measure(),
2353+
{
2354+
(i,): InstructionProperties(error=1e-3 * (num_qubits - i))
2355+
for i in range(num_qubits)
2356+
},
2357+
)
2358+
target.add_instruction(
2359+
CZGate(),
2360+
{
2361+
pair: InstructionProperties(error=1e-3 * sum(pair))
2362+
for pair in CouplingMap.from_line(num_qubits)
2363+
},
2364+
)
2365+
return generate_preset_pass_manager(
2366+
optimization_level=optimization_level, target=target, seed_transpiler=2025_10_27
2367+
)
2368+
2369+
def ghz(num_qubits):
2370+
qc = QuantumCircuit(num_qubits, num_qubits)
2371+
qc.h(0)
2372+
for i in range(1, num_qubits):
2373+
qc.cx(0, i)
2374+
qc.measure(qc.qubits, qc.clbits)
2375+
return qc
2376+
2377+
circuits = [ghz(5), ghz(8), ghz(10)]
2378+
shared_pm = make_pm()
2379+
shared_circuits = [shared_pm.run(circuit) for circuit in circuits]
2380+
own_circuits = [make_pm().run(circuit) for circuit in circuits]
2381+
self.assertEqual(shared_circuits, own_circuits)
2382+
together_circuits = make_pm().run(circuits)
2383+
self.assertEqual(together_circuits, own_circuits)
2384+
23362385

23372386
@ddt
23382387
class TestPostTranspileIntegration(QiskitTestCase):

test/python/transpiler/test_consolidate_blocks.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,24 @@ def test_collection_inside_control_flow(self):
728728
self.assertEqual(actual_outer.data[0].operation.blocks[0], block)
729729
self.assertEqual(actual_outer.data[1].operation.blocks[0].data[0].name, "unitary")
730730

731+
def test_pass_reuse(self):
732+
"""Test that the pass can be used more than once on different circuits."""
733+
# It matters that these have different numbers of qubits.
734+
hadamard = QuantumCircuit(1, 1)
735+
hadamard.h(0)
736+
hadamard.measure(0, 0)
737+
bell = QuantumCircuit(2, 2)
738+
bell.h(0)
739+
bell.cx(0, 1)
740+
bell.measure([0, 1], [0, 1])
741+
742+
def make_pass():
743+
return ConsolidateBlocks(force_consolidate=True)
744+
745+
shared = make_pass()
746+
self.assertEqual(shared(hadamard), make_pass()(hadamard))
747+
self.assertEqual(shared(bell), make_pass()(bell))
748+
731749
def test_invalid_python_data_does_not_panic(self):
732750
"""If a user feeds in invalid/old data, Rust space shouldn't panic."""
733751
# It doesn't really matter _what_ the failure mode is, just that we should return a regular

0 commit comments

Comments
 (0)