layout_method and routing_method selectors in transpile() #3999
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
ajavadia
requested review from
1ucian0,
diego-plan9,
ewinston,
itoko,
kdk,
lcapelluto,
mtreinish and
taalexander
as code owners
ajavadia
force-pushed
the
transpile-method-selectors
branch
from
ee86ad6 to
137b3f2
Compare
kdk
reviewed
Mar 25, 2020
There was a problem hiding this comment.
Looks good so far. Can you add a release note?
Right now, these don't completely override layout selection . E.g. for level2, if a user requests 'noise_adaptive', we'll still run csp_layout -> _choose_layout_condition -> noise_adaptive. This is reasonable behavior, but might not be what the user expects, so we should point it out in the docstring.
| _given_layout = SetLayout(initial_layout) | ||
|
|
||
| def _choose_layout_condition(property_set): | ||
| return not property_set['layout'] | ||
|
|
||
| _choose_layout = TrivialLayout(coupling_map) | ||
| if layout_method == 'trivial': |
There was a problem hiding this comment.
Can this be consolidated in e.g. _parse_layout_method (and pass the selected class or None in the passmanagerconfig?)
There was a problem hiding this comment.
yeah that would be good to avoid repetition but right now the layout passes don't have the same signature, so I can't just pass the class here.
if layout_method == 'trivial':
_choose_layout = TrivialLayout(coupling_map)
elif layout_method == 'dense':
_choose_layout = DenseLayout(coupling_map, backend_properties)
elif layout_method == 'noise_adaptive':
_choose_layout = NoiseAdaptiveLayout(backend_properties)It would be ideal to provide a unified interface though. I think in general there is too much repetition between the preset passmanagers, so a refactoring that makes them derive the same base and tweak would be great.
kdk
reviewed
Mar 25, 2020
ajavadia
force-pushed
the
transpile-method-selectors
branch
from
137b3f2 to
ebd290b
Compare
ajavadia
force-pushed
the
transpile-method-selectors
branch
from
a02e9c1 to
e88c19f
Compare
kdk
approved these changes
Mar 30, 2020
kdk
added
the
Changelog: API Change
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this pull request
Apr 7, 2020
In Qiskit#3999 we increased the number of trials, but the global benefit from doing this isn't clear. With the pending release weighing the increased run time for more trials against this, this commit temporarily reverts the number of trials for level 1 and level 2 back to their previous setting of 20. When we get to the bottom of Qiskit#4094 and have a fix or change for that we can increase the number of trials again. Related Qiskit#4094
ajavadia
pushed a commit
that referenced
this pull request
Apr 7, 2020
In #3999 we increased the number of trials, but the global benefit from doing this isn't clear. With the pending release weighing the increased run time for more trials against this, this commit temporarily reverts the number of trials for level 1 and level 2 back to their previous setting of 20. When we get to the bottom of #4094 and have a fix or change for that we can increase the number of trials again. Related #4094
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this pull request
Aug 5, 2020
* add ability to select layout and routing method from top transpile function * set lookahead width/depth in increasing order * releasenote * review comments * lint * lint Co-authored-by: Kevin Krsulich <[email protected]>
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this pull request
Aug 5, 2020
…4099) In Qiskit#3999 we increased the number of trials, but the global benefit from doing this isn't clear. With the pending release weighing the increased run time for more trials against this, this commit temporarily reverts the number of trials for level 1 and level 2 back to their previous setting of 20. When we get to the bottom of Qiskit#4094 and have a fix or change for that we can increase the number of trials again. Related Qiskit#4094
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.
Adding the ability to quickly select
layout_methodandrouting_methodfrom the top-leveltranspile()call. This configures the level 0-3 passmanagers to use the given methods for layout and routing.Something like this using
passmanager.replace()is also doable but not as convenient due to the issue of conditions, control-flow, etc. surrounding the passes.Generally I think there are a few basic & key steps to the compilation flow, each of which have several possible methods: optimization, placement, routing, scheduling, pulse selection.
With
optimization_level,layout_method,routing_methodwe cover the first 3. The 4th is in the form ofschedule(circuit, method)right now.Building a passmanager gives you ultimate control over how passes get chained, repeated until convergence, etc. but high-level options are still very useful in allowing the user to quickly explore alternatives. For example right now I don't think the "lookahead swap" is used anywhere or anyone knows how to invoke it.