Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize UnrollCustomDefinitions pass #10777

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

mtreinish
Copy link
Member

Summary

This commit makes some small tweaks to the UnrollCustomDefinitions pass to optimize it's runtime performance. It makes 2 primary changes to improve the efficiency. First, when calling circuit_to_dag() the copy_operations flag is set to False. This eliminates an inner deep copy from the conversion which will make the conversion operate more efficiently. The second is previously on every recursive call to run the pass we were previously reconstructing the pass on every execution. Instead this just reuses the existing instance. This will have less measurable of an impact as the pass construction is relatively fast, but it was unecessary work as we already have a pass object that can reused.

Details and comments

This commit makes some small tweaks to the UnrollCustomDefinitions pass
to optimize it's runtime performance. It makes 2 primary changes to
improve the efficiency. First, when calling `circuit_to_dag()` the
copy_operations flag is set to False. This eliminates an inner deep copy
from the conversion which will make the conversion operate more
efficiently. The second is previously on every recursive call to run the
pass we were previously reconstructing the pass on every execution.
Instead this just reuses the existing instance. This will have less
measurable of an impact as the pass construction is relatively fast, but
it was unecessary work as we already have a pass object that can reused.
@mtreinish mtreinish added performance Changelog: None Do not include in changelog labels Sep 5, 2023
@mtreinish mtreinish added this to the 0.45.0 milestone Sep 5, 2023
@mtreinish mtreinish requested a review from a team as a code owner September 5, 2023 21:25
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems correct - a transpiler pass is allowed to consider itself as owning the circuit that comes into it, so the copy=False is sound. Reusing self also just seems better in principle if nothing else - were we to be doing any per-run caching here, there's no reason not to share it between scopes.

@jakelishman jakelishman added this pull request to the merge queue Sep 5, 2023
Merged via the queue into Qiskit:main with commit 61e5bde Sep 6, 2023
@mtreinish mtreinish deleted the optimize-unroll-custom branch September 6, 2023 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog performance
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

3 participants