From bdbaa0dc34f6b6a0a8f62eb13d8c0d019e96db5b Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 18 Oct 2024 07:09:41 -0400 Subject: [PATCH] Run ConsolidateBlocks to build more blocks in init stage 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 #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 #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 #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). --- .../passes/optimization/consolidate_blocks.py | 17 ++++++++++++++--- .../preset_passmanagers/builtin_plugins.py | 8 +++++++- ...utside-basis-no-matrix-b492437b18366435.yaml | 11 +++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/fix-consolidate-blocks-gates-outside-basis-no-matrix-b492437b18366435.yaml diff --git a/qiskit/transpiler/passes/optimization/consolidate_blocks.py b/qiskit/transpiler/passes/optimization/consolidate_blocks.py index 49f227e8a746..e2fc8d9a6bfa 100644 --- a/qiskit/transpiler/passes/optimization/consolidate_blocks.py +++ b/qiskit/transpiler/passes/optimization/consolidate_blocks.py @@ -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 @@ -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: diff --git a/qiskit/transpiler/preset_passmanagers/builtin_plugins.py b/qiskit/transpiler/preset_passmanagers/builtin_plugins.py index 68e266a09e70..1c76e8719ae5 100644 --- a/qiskit/transpiler/preset_passmanagers/builtin_plugins.py +++ b/qiskit/transpiler/preset_passmanagers/builtin_plugins.py @@ -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 diff --git a/releasenotes/notes/fix-consolidate-blocks-gates-outside-basis-no-matrix-b492437b18366435.yaml b/releasenotes/notes/fix-consolidate-blocks-gates-outside-basis-no-matrix-b492437b18366435.yaml new file mode 100644 index 000000000000..7be97f2b86c7 --- /dev/null +++ b/releasenotes/notes/fix-consolidate-blocks-gates-outside-basis-no-matrix-b492437b18366435.yaml @@ -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.