Speed up synthesis passes when there is nothing to synthesize #10788
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit makes a small optimization to the UnitarySynthesis and HighLevelSynthesis synthesis passes when there are no gates to synthesize. Previously these passes would iterate over the entire DAG and check every gate to determine that None needed to be synthesized. While this is relatively quick for very large circuits this can take 100s of ms. However, this was wasted work because the DAG carries a dictionary of op names in the circuit so we can determine in constant time whether any of the operations to synthesize are present up front and skip the bulk of the iteration. To improve the impact here the DAGCircuit.count_ops() method is updated to skip the recursion if no control flow operations are present. Otherwise we'd still be iterating over the DAG (in a faster manner) which would limit the improvement from this commit. This isn't the best approach for that as it uses a hardcoded list of control flow operations which doesn't seem super general purpose, but as this hard coded set of instructions is already used elsewhere in the DAGCircuit this isn't making the situation any worse. If/when we expand recursive objects this can be revisited (and we'll likely not be able to avoid the iteration in count ops). With these changes the time these passes take when there are no operations to synthesize only takes ~5-15 microseconds. A similar optimization should be considered for UnrollCustomDefinitions as this takes an even longer amount of time to iterate when there is nothing to translate. However, it wasn't done as part of this PR as to check the equivalence library has an entry we need to work with gate objects (well actually name and number of qubits as that's all that's used from the object for a lookup) and the name from the op count dictionary is not sufficient for a lookup. So this will need to be handled independently in a separate PR.
mtreinish
added
performance
Changelog: None
mtreinish
requested review from
alexanderivrii,
ShellyGarion and
a team
as code owners
|
One or more of the the following people are requested to review this:
|
jakelishman
reviewed
Sep 6, 2023
There was a problem hiding this comment.
This seems like a generally good idea to me. The control-flow ops being somewhat hardcoded is a little sub-optimal, but then so's everything about our handling of control-flow operations within the DAG IR, so I'm not desperately worried about it. We can revisit if/when we improve our representation of them.
jakelishman
approved these changes
Sep 7, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This commit makes a small optimization to the UnitarySynthesis and HighLevelSynthesis synthesis passes when there are no gates to synthesize. Previously these passes would iterate over the entire DAG and check every gate to determine that None needed to be synthesized. While this is relatively quick for very large circuits this can take 100s of ms. However, this was wasted work because the DAG carries a dictionary of op names in the circuit so we can determine in constant time whether any of the operations to synthesize are present up front and skip the bulk of the iteration. To improve the impact here the DAGCircuit.count_ops() method is updated to skip the recursion if no control flow operations are present. Otherwise we'd still be iterating over the DAG (in a faster manner) which would limit the improvement from this commit. This isn't the best approach for that as it uses a hardcoded list of control flow operations which doesn't seem super general purpose, but as this hard coded set of instructions is already used elsewhere in the DAGCircuit this isn't making the situation any worse. If/when we expand recursive objects this can be revisited (and we'll likely not be able to avoid the iteration in count ops). With these changes the time these passes take when there are no operations to synthesize only takes ~10-20 microseconds.
A similar optimization should be considered for UnrollCustomDefinitions as this takes an even longer amount of time to iterate when there is nothing to translate. However, it wasn't done as part of this PR as to check the equivalence library has an entry we need to work with gate objects (well actually name and number of qubits as that's all that's used from the object for a lookup) and the name from the op count dictionary is not sufficient for a lookup. So this will need to be handled independently in a separate PR.
Details and comments