Avoid circuit<->dag conversion overhead in basis translator #9680
Assignees
Labels
Comments
4 tasks
3 tasks
|
Some thoughts, copied from #10275: I'm not certain that giving |
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this issue
Aug 24, 2023
This commit updates the internal logic for doing basis translation with equivalence rules that involve parameter expressions. Previously the basis translator would first construct an unbound DAGCircuit from the equivalence library search, the it would convert that DAGCircuit into a QuantumCircuit, bind the parameters, and finally convert the QuantumCircuit back to a DAGCircuit for inline replacement. This was needlessly inefficient, especially considering the original unbound DAGCircuit was built from a QuamtumCircuit object. This meant in the case of each gate being translated with a parameterized equivalence rule the BasisTranslator was doing: QuantumCircuit -> DAGCircuit -> QuantumCircuit -> DAGCircuit just to be able to replace a node. This ends up being the primary performance bottleneck for the pass because while each conversion of a typically very small circuit is fairly quick in aggregate it takes a large amount of time as the number of gates in the circuit grows. This commit attempts to address this performance overhead by removing the last QuantumCircuit -> DAGCircuit conversion by directly binding the parameters on a copy of the DAGCircuit. Since the equivalence library use case is very constrained in how parameter expressions are constructed we're able to inline the binding on a copy of the DAGCircuit. This still has a fair amount of overhead as we still need to iterate over the DAGCircuit and copy it, but we're able to avoid one set of copies and circuit iteration by doing the binding this way. Fixes Qiskit#9680
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 18, 2023
* Inline parameter binding as part of BasisTranslator This commit updates the internal logic for doing basis translation with equivalence rules that involve parameter expressions. Previously the basis translator would first construct an unbound DAGCircuit from the equivalence library search, the it would convert that DAGCircuit into a QuantumCircuit, bind the parameters, and finally convert the QuantumCircuit back to a DAGCircuit for inline replacement. This was needlessly inefficient, especially considering the original unbound DAGCircuit was built from a QuamtumCircuit object. This meant in the case of each gate being translated with a parameterized equivalence rule the BasisTranslator was doing: QuantumCircuit -> DAGCircuit -> QuantumCircuit -> DAGCircuit just to be able to replace a node. This ends up being the primary performance bottleneck for the pass because while each conversion of a typically very small circuit is fairly quick in aggregate it takes a large amount of time as the number of gates in the circuit grows. This commit attempts to address this performance overhead by removing the last QuantumCircuit -> DAGCircuit conversion by directly binding the parameters on a copy of the DAGCircuit. Since the equivalence library use case is very constrained in how parameter expressions are constructed we're able to inline the binding on a copy of the DAGCircuit. This still has a fair amount of overhead as we still need to iterate over the DAGCircuit and copy it, but we're able to avoid one set of copies and circuit iteration by doing the binding this way. Fixes #9680 * Handle parameter substition individually In the case we're substituting a parameter expression in the replacement circuit with another parameter expression we need to handle each sub parameter in the expression individually to either bind it to a value or substitute it with the replacement expression. Previously we tried to either call ParameterExpression.bind or ParameterExpression.subs, but subs can only be used if all the parameters in an expression are unbound. This commit replaces the use of subs with a loop over the parameters and calls ParameterExpression.assign, which handles the type based dispatch for a single parameter. The case when the full substitution is bound is handled using bind as before, since that can be done in a single call. * Update qiskit/transpiler/passes/basis/basis_translator.py Co-authored-by: Julien Gacon <[email protected]> * Use zip instead of zip_longest * Adjust global phase type casting --------- Co-authored-by: Julien Gacon <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What should we add?
Right now in the basis translator we spend a large portion of time (roughly 1/3rd of the typical runtime) converting back and forth between
QuantumCircuitandDAGCircuit. This is mostly of this is to support callingassign_parameters()which only exists on a circuit, but the transpiler only works with DAGCircuit. This ends up being fairly high overhead because we deepcopy instructions multiple times as we go back and forthWe should be able to avoid all this overhead by doing parameter assignment directly in the
DAGCircuitwhich would probably require porting the equivalent ofParameterTableand the matching assignment functions toDAGCircuit(or potentially bypassing the traditional parameter assignment mechanisms as we don't need the full flexibility for this).The text was updated successfully, but these errors were encountered: