Only run translation stage in optimization loop if necessary #6940
Conversation
This commit adds a new analysis pass, GatesInBasis, which is used to check if all the gates in the circuit are currently in the basis set or not. The intent for this pass is to use this to conditionally trigger the execution of basis translation (normally the BasisTranslator) in the main optimization loop of the preset pass managers, however this is blocked on Qiskit#6830. So for now this just adds the pass so we can use it in the future.
irajput
changed the title
|
As we discussed elsewhere, at first glance, it looks like #6938 solves the problem of nesting conditionals in a more general way. I mean, you can do more with |
irajput
changed the title
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
|
The blocker #6962 has been merged. But, I am leaving the "hold" tag because there is a new error, probably because of an incompatible change in one of the recent merges of the main branch. |
There was a problem hiding this comment.
The CI failures are intest.python.providers.test_backend_v2.TestBackendV2 ( https://dev.azure.com/qiskit-ci/qiskit-terra/_build/results?buildId=33780&view=logs&j=5ed7f5be-2954-510c-f69f-57064fd91e48&t=42b17b83-3671-53ec-2b04-ff846dcbcf0e&l=16157 ).
I'm not 100% sure, but it looks like this is preventing a run of the BasisTranslator (which is needed here for the BackendV2 case where not all gates are present on all qubits/pairs of qubits). I think this can be resolved by making GatesInBasis "target-aware" but maybe @mtreinish could say more definitively (or if this would be better handled in a seperate issue).
Yeah, we'll need to update the |
HuangJunye
added
the
Community PR
This commit expands the conditional unrolling so that it occurs in every optimization level that has an optimization loop. Since this PR branch was originally created we've expanded the use of the unroller stage in the optimization loop to account for other issues caused by new developments in the transpiler since this was originally developed. This commit updates level 1 and level 2 to avoid unecessary basis translation if the circuit is already in the target basis.
For the gates in basis pass it previously was raising an error if you initialized the pass without a target or basis gates constraint set. This was because the pass didn't have any constraints to enforce so there wasn't anything to validate. However, with the integration of the pass into the preset pass managers this behavior wasn't desireable because people run transpile() without a backend, target, or basis_gates set. To account for this the pass logic is changed to just return True if there are no instruction contstraints because all the gates are in the basis since there is no basis.
mtreinish
changed the title
|
I've updated this PR so I think it's ready to merge now, but since I basically rewrote it as part of updating the PR, I'd like to get another set of eyes to review it before we merge. |
mtreinish
added
performance
mod: transpiler
| nonlocal gates_in_basis_true_count | ||
| nonlocal collect_2q_blocks_count |
There was a problem hiding this comment.
It's not everyday there's a ~valid usage for nonlocal.
Summary
This commit updates the preset pass managers to only run the translation stage as part of the optimization loop if
there are gates outside the target in the circuit. This leverages the
GatesInBasisanalysis pass to the defaultoptimization loops for preset passmanagers level 1, 2, and 3 and conditions the execution of the basis translation passes
on whether any gates are detected outside the target basis. This should also address the potential issue reported in
#6677 where additional translation is resulting in unoptimized output in some cases.
Details and comments
Fixes #6677