-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Set max_trials
for VF2Layout
in preset pass managers.
#10054
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
1814a5a
Set max_trials for VF2Layout in preset pass managers.
kevinhartman 9c0e5e8
Address review comments.
kevinhartman 0e315eb
Return tuple of None instead for finer control and a better interface.
kevinhartman 9a6a4ca
Add deprecation notice to release note.
kevinhartman 5ef7a94
Remove deprecation until 0.25.0.
kevinhartman 89148c9
Remove unused import.
kevinhartman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 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 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 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 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
23 changes: 23 additions & 0 deletions
23
releasenotes/notes/preset-pm-vf2-max-trials-958bb8a36fff472f.yaml
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
--- | ||
upgrade: | ||
- | | ||
The maximum number of trials evaluated when searching for the best | ||
layout using :class:`.VF2Layout` and :class:`.VF2PostLayout` is now | ||
limited in | ||
:func:`qiskit.transpiler.preset_passmanagers.level_1_pass_manager`, | ||
:func:`qiskit.transpiler.preset_passmanagers.level_2_pass_manager`, | ||
and | ||
:func:`qiskit.transpiler.preset_passmanagers.level_3_pass_manager` | ||
to ``2,500``, ``25,000``, and ``250,000``, respectively. Previously, | ||
all possible layouts were evaluated. This change was made to prevent | ||
transpilation from hanging during layout scoring for circuits with many | ||
connected components on larger devices, which scales combinatorially | ||
since each connected component must be evaluated in all possible | ||
positions on the device. To perform a full search as | ||
before, manually run :class:`.VF2PostLayout` over the transpiled circuit | ||
in strict mode, specifying ``0`` for ``max_trials``. | ||
fixes: | ||
- | | ||
Fixed a potential performance scaling issue with layout scoring in preset | ||
pass managers, which could occur when transpiling circuits with many | ||
connected components on large devices. |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the need to count zeros is good. Maybe
25000
is near the readability threshold, but I'd still opt for making it more readable. Moreso for250000
. Some options areIt's not obvious to me whether the LHS or RHS of
30_000_000 == int(3e7)
is more readable. Some researchers measure comprehension speed with clocks, but I couldn't find a study on this specific case. I guess in this file it makes sense to use one of either underscores orint
uniformly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case I personally don't view it as a big deal, the number here is just an arbitrary limit that was found by experimentation, the actual numeric value isn't really relevant. My personal preference would be for
2.5e5
as that is how I reason about this in my head because it's more about the order of magnitude between each optimization level. But I can personally see that just as well with how it's written now (not so much by the absolute zero count but by the relative spacing differences). What I view as far more important here is the comment explaining where the value came from because that's what we're really setting is a wall time limit as a second order effect.