Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support control flow in ConsolidateBlocks #10355

Merged
merged 30 commits into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
413aa02
Support control flow in ConsolidateBlocks
jlapeyre Jun 28, 2023
c882c6a
Add release note for support control flow in ConsolidateBlocks
jlapeyre Jun 28, 2023
d269db5
Move imports from inside function to top level
jlapeyre Jun 29, 2023
c32d69c
Make construction of ConsolidateBlocks for control flow ops more effi…
jlapeyre Jun 29, 2023
d4bc8bd
Do IfElseOp test without builder interface
jlapeyre Jul 10, 2023
d210512
Linting
jlapeyre Jul 10, 2023
eabaee5
Avoid cyclic import
jlapeyre Jul 10, 2023
c567d0d
Try to fix cyclic import (in lint tool only)
jlapeyre Jul 10, 2023
7a34941
Reuse top-level consolidation pass and add tests
jlapeyre Jul 11, 2023
3da4990
Remove argument `decomposer` from constructor of ConsolidateBlocks
jlapeyre Jul 11, 2023
e4a39fe
Merge branch 'main' into controlflow/consolidate_blocks
jlapeyre Jul 11, 2023
248479d
Remove cruft accidentally left
jlapeyre Jul 11, 2023
dc69654
Move function-level import to module level
jlapeyre Jul 11, 2023
60e27b4
Write loop more concisely
jlapeyre Jul 12, 2023
8bea1e7
Update releasenotes/notes/add-control-flow-to-consolidate-blocks-e013…
jlapeyre Jul 12, 2023
87b979e
Use assertion in tests with better diagnostics
jlapeyre Jul 12, 2023
bf0a1f4
Remove reference to decomposer from docstring to ConsolidateBlocks
jlapeyre Jul 12, 2023
d3d2925
Use more informative tests for ConsolidateBlocks with control flow
jlapeyre Jul 19, 2023
a5b57c1
Merge branch 'main' into controlflow/consolidate_blocks
jlapeyre Jul 19, 2023
c24f63e
Simplify test in test_consolidate_blocks and factor
jlapeyre Jul 19, 2023
2ae8558
Factor more code in test
jlapeyre Jul 19, 2023
c077e49
Use clbit in circuit as test bit when appending IfElse
jlapeyre Jul 19, 2023
7cef53b
In test, use bits in circuit as specifiers when appending gate to tha…
jlapeyre Jul 19, 2023
d4460a5
In a test, don't use qubits from unrelated circuit when constructing …
jlapeyre Jul 19, 2023
54bd958
Factor code in test to simplify test
jlapeyre Jul 19, 2023
be2a65f
Merge branch 'main' into controlflow/consolidate_blocks
jlapeyre Jul 19, 2023
8eae8c3
Simplify remaining control flow op tests for ConsolidateBlocks
jlapeyre Jul 19, 2023
b36a949
Run black
jlapeyre Jul 20, 2023
5dc5f85
Update test/python/transpiler/test_consolidate_blocks.py
jlapeyre Jul 20, 2023
196beb7
Merge branch 'main' into controlflow/consolidate_blocks
jlapeyre Jul 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions qiskit/transpiler/passes/optimization/consolidate_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@
from qiskit.extensions import UnitaryGate
from qiskit.circuit.library.standard_gates import CXGate
from qiskit.transpiler.basepasses import TransformationPass
from qiskit.circuit.controlflow import ControlFlowOp
from qiskit.transpiler.passmanager import PassManager
from qiskit.transpiler.passes.synthesis import unitary_synthesis
from .collect_1q_runs import Collect1qRuns
from .collect_2q_blocks import Collect2qBlocks


class ConsolidateBlocks(TransformationPass):
Expand All @@ -49,12 +53,16 @@ def __init__(
):
"""ConsolidateBlocks initializer.

If `kak_basis_gate` is not `None` it will be used as the basis gate for KAK decomposition.
Otherwise, if `basis_gates` is not `None` a basis gate will be chosen from this list.
Otherwise the basis gate will be `CXGate`.

Args:
kak_basis_gate (Gate): Basis gate for KAK decomposition.
force_consolidate (bool): Force block consolidation
force_consolidate (bool): Force block consolidation.
basis_gates (List(str)): Basis gates from which to choose a KAK gate.
approximation_degree (float): a float between [0.0, 1.0]. Lower approximates more.
target (Target): The target object for the compilation target backend
target (Target): The target object for the compilation target backend.
"""
super().__init__()
self.basis_gates = None
Expand Down Expand Up @@ -159,11 +167,32 @@ def run(self, dag):
dag.remove_op_node(node)
else:
dag.replace_block_with_op(run, unitary, {qubit: 0}, cycle_check=False)

dag = self._handle_control_flow_ops(dag)

# Clear collected blocks and runs as they are no longer valid after consolidation
if "run_list" in self.property_set:
del self.property_set["run_list"]
if "block_list" in self.property_set:
del self.property_set["block_list"]

return dag

def _handle_control_flow_ops(self, dag):
"""
This is similar to transpiler/passes/utils/control_flow.py except that the
collect blocks is redone for the control flow blocks.
"""

pass_manager = PassManager()
if "run_list" in self.property_set:
pass_manager.append(Collect1qRuns())
if "block_list" in self.property_set:
pass_manager.append(Collect2qBlocks())
Comment on lines +187 to +191
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can likely make this PassManager just once up in __init__ and retrieve it from self each time, even during the recursion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. The cost should be negligible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After trying, I don't see an easy way to make this work. A ConsolidateBlocks instance is constructed before the top-level RunningPassManager injects the necessary information. The call PassManager on my machine takes < 250ns. Constructing it and checking a dict and appending passes takes 13us or 26us.

We could pass the required information when constructing ConsolidateBlocks. That would allow us to do this more cleanly in the future. Maybe populate requires based boolean-valued arguments. We'd support the status quo for a while.

In fact, doing this would be less fragile and better signal intent than the status quo. This:

https://github.com/Qiskit/qiskit-terra/blob/fb9d5d8bb41f1e289b5ee895ea087cc92e74a921/qiskit/transpiler/preset_passmanagers/level3.py#L174-L179

is not as transparent as

        ConsolidateBlocks(
            collect_2q_blocks=True, basis_gates=basis_gates, target=target, approximation_degree=approximation_degree
        ),
        UnitarySynthesis(
            basis_gates,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing the required information to ConsolidateBlocks would be my general choice so that the top level of the recursion and the recursive calls can all look more similar. At this stage in the 0.25 cycle, it might be better to leave what you've got and fix it in 0.45 instead so we've got more time.


pass_manager.append(self)
for node in dag.op_nodes(ControlFlowOp):
node.op = node.op.replace_blocks(pass_manager.run(block) for block in node.op.blocks)
return dag

def _check_not_in_basis(self, gate_name, qargs, global_index_map):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
features:
- |
Enabled performing the :class:`.ConsolidateBlocks` pass inside the
blocks of :class:`.ControlFlowOp`. This pass collects several sequences of gates
and replaces each sequence with the equivalent numeric unitary gate. This new feature enables
applying this pass recursively to the blocks in control flow operations. Note that the meaning
of "block" in :class:`.ConsolidateBlocks` is unrelated to that in
:class:`.ControlFlowOp`.
130 changes: 128 additions & 2 deletions test/python/transpiler/test_consolidate_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import unittest
import numpy as np

from qiskit.circuit import QuantumCircuit, QuantumRegister
from qiskit.circuit.library import U2Gate, SwapGate, CXGate
from qiskit.circuit import QuantumCircuit, QuantumRegister, IfElseOp
from qiskit.circuit.library import U2Gate, SwapGate, CXGate, CZGate
from qiskit.extensions import UnitaryGate
from qiskit.converters import circuit_to_dag
from qiskit.transpiler.passes import ConsolidateBlocks
Expand Down Expand Up @@ -428,6 +428,132 @@ def test_identity_1q_unitary_is_removed(self):
pm = PassManager([Collect2qBlocks(), Collect1qRuns(), ConsolidateBlocks()])
self.assertEqual(QuantumCircuit(5), pm.run(qc))

def test_descent_into_control_flow(self):
"""Test consolidation in blocks when control flow op is the same as at top level."""

def circuit_of_test_gates():
qc = QuantumCircuit(2, 1)
qc.cx(0, 1)
qc.cx(1, 0)
return qc

def do_consolidation(qc):
pass_manager = PassManager()
pass_manager.append(Collect2qBlocks())
pass_manager.append(ConsolidateBlocks(force_consolidate=True))
return pass_manager.run(qc)

result_top = do_consolidation(circuit_of_test_gates())

qc_control_flow = QuantumCircuit(2, 1)
ifop = IfElseOp((qc_control_flow.clbits[0], False), circuit_of_test_gates(), None)
qc_control_flow.append(ifop, qc_control_flow.qubits, qc_control_flow.clbits)

result_block = do_consolidation(qc_control_flow)
gate_top = result_top[0].operation
gate_block = result_block[0].operation.blocks[0][0].operation
np.testing.assert_allclose(gate_top, gate_block)

def test_not_crossing_between_control_flow_block_and_parent(self):
"""Test that consolidation does not occur across the boundary between control flow
blocks and the parent circuit."""
qc = QuantumCircuit(2, 1)
qc.cx(0, 1)
qc_true = QuantumCircuit(2, 1)
qc_false = QuantumCircuit(2, 1)
qc_true.cx(0, 1)
qc_false.cz(0, 1)
ifop = IfElseOp((qc.clbits[0], True), qc_true, qc_false)
qc.append(ifop, qc.qubits, qc.clbits)

pass_manager = PassManager()
pass_manager.append(Collect2qBlocks())
pass_manager.append(ConsolidateBlocks(force_consolidate=True))
qc_out = pass_manager.run(qc)

self.assertIsInstance(qc_out[0].operation, UnitaryGate)
np.testing.assert_allclose(CXGate(), qc_out[0].operation)
op_true = qc_out[1].operation.blocks[0][0].operation
op_false = qc_out[1].operation.blocks[1][0].operation
np.testing.assert_allclose(CXGate(), op_true)
np.testing.assert_allclose(CZGate(), op_false)

def test_not_crossing_between_control_flow_ops(self):
"""Test that consolidation does not occur between control flow ops."""
qc = QuantumCircuit(2, 1)
qc_true = QuantumCircuit(2, 1)
qc_false = QuantumCircuit(2, 1)
qc_true.cx(0, 1)
qc_false.cz(0, 1)
ifop1 = IfElseOp((qc.clbits[0], True), qc_true, qc_false)
qc.append(ifop1, qc.qubits, qc.clbits)
ifop2 = IfElseOp((qc.clbits[0], True), qc_true, qc_false)
qc.append(ifop2, qc.qubits, qc.clbits)

pass_manager = PassManager()
pass_manager.append(Collect2qBlocks())
pass_manager.append(ConsolidateBlocks(force_consolidate=True))
qc_out = pass_manager.run(qc)

op_true1 = qc_out[0].operation.blocks[0][0].operation
op_false1 = qc_out[0].operation.blocks[1][0].operation
op_true2 = qc_out[1].operation.blocks[0][0].operation
op_false2 = qc_out[1].operation.blocks[1][0].operation
np.testing.assert_allclose(CXGate(), op_true1)
np.testing.assert_allclose(CZGate(), op_false1)
np.testing.assert_allclose(CXGate(), op_true2)
np.testing.assert_allclose(CZGate(), op_false2)

jlapeyre marked this conversation as resolved.
Show resolved Hide resolved
def test_inverted_order(self):
"""Test that the `ConsolidateBlocks` pass creates matrices that are correct under the
application of qubit binding from the outer circuit to the inner block."""
body = QuantumCircuit(2, 1)
body.h(0)
body.cx(0, 1)

id_op = Operator(np.eye(4))
bell = Operator(body)

qc = QuantumCircuit(2, 1)
# The first two 'if' blocks here represent exactly the same operation as each other on the
# outer bits, because in the second, the bit-order of the block is reversed, but so is the
# order of the bits in the outer circuit that they're bound to, which makes them the same.
# The second two 'if' blocks also represnt the same operation as each other, but the 'first
# two' and 'second two' pairs represent qubit-flipped operations.
qc.if_test((0, False), body.copy(), qc.qubits, qc.clbits)
qc.if_test((0, False), body.reverse_bits(), reversed(qc.qubits), qc.clbits)
qc.if_test((0, False), body.copy(), reversed(qc.qubits), qc.clbits)
qc.if_test((0, False), body.reverse_bits(), qc.qubits, qc.clbits)

# The first two operations represent Bell-state creation on _outer_ qubits (0, 1), the
# second two represent the same creation, but on outer qubits (1, 0).
expected = [
id_op.compose(bell, qargs=(0, 1)),
id_op.compose(bell, qargs=(0, 1)),
id_op.compose(bell, qargs=(1, 0)),
id_op.compose(bell, qargs=(1, 0)),
]

actual = []
pm = PassManager([Collect2qBlocks(), ConsolidateBlocks(force_consolidate=True)])
for instruction in pm.run(qc).data:
# For each instruction, the `UnitaryGate` that's been created will always have been made
# (as an implementation detail of `DAGCircuit.collect_2q_runs` as of commit e5950661) to
# apply to _inner_ qubits (0, 1). We need to map that back to the _outer_ qubits that
# it applies to compare.
body = instruction.operation.blocks[0]
wire_map = {
inner: qc.find_bit(outer).index
for inner, outer in zip(body.qubits, instruction.qubits)
}
actual.append(
id_op.compose(
Operator(body.data[0].operation),
qargs=[wire_map[q] for q in body.data[0].qubits],
)
)
self.assertEqual(expected, actual)


if __name__ == "__main__":
unittest.main()