Skip to content

Commit

Permalink
Run ConsolidateBlocks to build more blocks in init stage
Browse files Browse the repository at this point in the history
Right now when we're running the consolidate blocks pass during the init
stage we were not consolidating all 2q blocks found. This was because
the pass was not being called with the target's information and it
wasn't aware when the gates in a block were outside of the target. To
avoid potentially unecessary work the ConsolidateBlocks pass has some
logic to avoid collecting into a block if it doesn't think that the
unitary decomposition can decrease the 2q gate count, basically trying
to avoid the case where the synthesis output would be more expensive
than original circuit gates. However if the gates are outside of the
target it can't estimate the synthesis cost and should be consolidating
the block. The logic for this exists in the pass, but it only works if
you specify the target so it knows that there are gates in the block
outside the target.

As part of this a bug around handling gates outside basis in the
pass was fixed. If gates were encountered that had no matrix defined
the pass would previously have errored trying to create a unitary gate
for the block in some cases.

In general the logic around this pass is lacking in several cases (see
issue Qiskit#11659) and it would be better to estimate the error of the block
and determine if the estimated error from the output of different
synthesis alternatives is better or not. This is easy to do if we do
consolidation and synthesis in a single step. But for this situation of
running consolidation during init stage and synthesis much later during
the translation stage this is hard to accomplish because we lose the
context of the original block if we consolidate. That is why we should
consider changing the init stage's use of `ConsolidateBlocks` to set
`force_consolidate=True` which will create a `UnitaryGate` out of all
2q blocks found regardless of the estimated number of KAK gates needed
based on the unitary matrix's coordinates in the weyl chamber. Doing
this would fix issues like Qiskit#13344 because we'll synthesisze the unitary
in that case. Where it's less clear we want to do this though is cases
where the target has different 2q gates than the KAK gate synthesis
knows how to work with. Take the example of Qiskit#13344, if the target had
global supported gates of `[cp, u, cx]` or something like that and the
angle was not zero then forcing consolidation would change a single cp
into possibly 3 cx gates. So this commit does not set
`force_consolidate=True` and we can discuss the best way to handle that
in a separate commit (either on this PR or a new one).
  • Loading branch information
mtreinish committed Oct 18, 2024
1 parent 68a1eca commit bdbaa0d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
17 changes: 14 additions & 3 deletions qiskit/transpiler/passes/optimization/consolidate_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,12 @@ def run(self, dag):
all_block_gates = set()
for block in blocks:
if len(block) == 1 and self._check_not_in_basis(dag, block[0].name, block[0].qargs):
try:
matrix = block[0].op.to_matrix()
except QiskitError:
continue
all_block_gates.add(block[0])
dag.substitute_node(block[0], UnitaryGate(block[0].op.to_matrix()))
dag.substitute_node(block[0], UnitaryGate(matrix))
else:
basis_count = 0
outside_basis = False
Expand Down Expand Up @@ -158,10 +162,17 @@ def run(self, dag):
if any(gate in all_block_gates for gate in run):
continue
if len(run) == 1 and not self._check_not_in_basis(dag, run[0].name, run[0].qargs):
dag.substitute_node(run[0], UnitaryGate(run[0].op.to_matrix(), check_input=False))
try:
matrix = run[0].op.to_matrix()
except QiskitError:
continue
dag.substitute_node(run[0], UnitaryGate(matrix, check_input=False))
else:
qubit = run[0].qargs[0]
operator = run[0].op.to_matrix()
try:
operator = run[0].op.to_matrix()
except QiskitError:
continue
already_in_block = False
for gate in run[1:]:
if gate in all_block_gates:
Expand Down
8 changes: 7 additions & 1 deletion qiskit/transpiler/preset_passmanagers/builtin_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,13 @@ def pass_manager(self, pass_manager_config, optimization_level=None) -> PassMana
)
init.append(CommutativeCancellation())
init.append(Collect2qBlocks())
init.append(ConsolidateBlocks())
init.append(
ConsolidateBlocks(
approximation_degree=pass_manager_config.approximation_degree,
target=pass_manager_config.target,
basis_gates=pass_manager_config.basis_gates,
)
)
# If approximation degree is None that indicates a request to approximate up to the
# error rates in the target. However, in the init stage we don't yet know the target
# qubits being used to figure out the fidelity so just use the default fidelity parameter
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
fixes:
- |
Fixed an issue in the :class:`.ConsolidateBlocks` transpiler pass that
would cause it to error if it encountered a gate outside the specified
``target`` or ``basis_gates`` that didn't have a matrix defined in a
standalone 2q block. In these cases previously the pass would attempt to
consolidate the gate into a :class:`.UnitaryGate` and error because it
needed the gates matrix. This has been fixed so that those custom gates
are ignored for the purposes of the pass because it is unable to process
a gate numerically if it's matrix isn't defined.

0 comments on commit bdbaa0d

Please sign in to comment.