-
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
Always try initial layout as first guess in preset passmanagers #5702
Always try initial layout as first guess in preset passmanagers #5702
Conversation
This is failing tests because it doesn't look like the setlayout pass is running at all to reset the layout to none after trivial is found to not be perfect. In the test that's failing csplayout doesn't converge so layout is never getting reset and dense layout is skipped because we still have the trivial layout, so the trivial layout passes through to the end (which breaks the expected result). I'm not sure how to get layout set to None in the property set based on the conditional output of a previous analysis pass. I tried to do it in the condition logic, but the fencing got it the way of that approach. |
Reading through the conditions here has become pretty tricky, so I'm not 100% sure I'm parsing the situation correctly. If we want to extend the Otherwise, we have a mix of cases where sometimes |
In the preset passmanagers level2 and level3 the initial layout is generated by the CSP layout pass and then if an answer can't be found the specified layout method is used instead. However, as was reported in issue Qiskit#5694 the CSP layout completely misses when the trivial layout case perfectly maps the input circuit to the device. This results in level 1 significantly out performing level2 and level3 because it has to go to routing and inserts a lot of swaps. To address this hole in the CSP layout case this commit updates the preset passmanagers for level 2 and level 3 to always try a trivial layout first, if this doesn't result in a perfect mapping the pass is unchanged it will use CSP layout and then the configured layout method. Fixes Qiskit#5694
24844a4
to
fb10b69
Compare
It's a little more involved than that because we can't rely on Layout2qDistance not being perfect after csp layout because we want to use csp's output if the pass converged on a solution and trivial wasn't perfect. If trivial isn't perfect and csp didn't converge then we should use dense layout. But this put me on the right track, instead of using layout2qdistance I looked for the csp stop reason and used that as the condition for running dense. See: fb10b69 |
"best of" passmanager flow controller (see PR #3018) is a similar solution. Do you mind if I tag |
I'd rather we leave it as is, if the best of PR merges before this I can refactor this PR on top of it to switch the logic to use that instead. |
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.
Following @1ucian0 's comment in #5694 (comment) , this should probably wait for something like PassManager.best_of
, specifically because there are cases where we wouldn't want this behavior.
For example, a GHZ circuit can be laid out "perfectly" in a handful of ways on a large device, so having the ability to find the one which minimizes expected noise is valuable. #5694 seems a less common case (where the user could use layout_method='trivial'
given their knowledge of the circuit shape) and while we should find the trivial layout when the cost savings are this substantial, I wouldn't want to lose the former for the latter today. (Hopefully, PassManager.best_of
gives us the ability to have both.)
I'm removing the stable backport tag on this, mostly because I think we should let it sit on master a bit so we can get some benchmark runs to see the impact of this since we're plannign a release shortly. If it all looks good in a couple weeks we can add the tag back and include it for a 0.17.2 release. |
Summary
In the preset passmanagers level2 and level3 the initial layout is
generated by the CSP layout pass and then if an answer can't be found
the specified layout method is used instead. However, as was reported in
issue #5694 the CSP layout completely misses when the trivial layout case
perfectly maps the input circuit to the device. This results in level 1
significantly out performing level2 and level3 because it has to go to
routing and inserts a lot of swaps. To address this hole in the CSP
layout case this commit updates the preset passmanagers for level 2 and
level 3 to always try a trivial layout first, if this doesn't result in
a perfect mapping the pass is unchanged it will use CSP layout and then
the configured layout method.
Details and comments
Fixes #5694