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

Increase heuristic effort for sabre #10588

Closed
wants to merge 5 commits into from

Conversation

mtreinish
Copy link
Member

Summary

This commit increases the heuristic effort for sabre layout and routing. This is made through 2 changes, the first is the depth of the internal lookahead heuristic used in sabre swap has been increased from 20 to 72. This is just a stop-gap for a potential reworking of the lookahead heuristic. In local testing for larger backends and deeper circuits in this is showing better output results than the current default of 20 without any runtime impact. The other aspect is that the trial counts for each optimization level > 0 is increased 5x. This has a runtime cost, but it the performance of the rust sabre implementation is fast enough that even running the algorithm with more trials it is not a bottleneck for typical compilation.

Details and comments

Related to #10160

This commit increases the heuristic effort for sabre layout and routing.
This is made through 2 changes, the first is the depth of the internal
lookahead heuristic used in sabre swap has been increased from 20 to 72.
This is just a stop-gap for a potential reworking of the lookahead
heuristic. In local testing for larger backends and deeper circuits in
this is showing better output results than the current default of 20
without any runtime impact. The other aspect is that the trial counts
for each optimization level > 0 is increased 5x. This has a runtime cost,
but it the performance of the rust sabre implementation is fast enough
that even running the algorithm with more trials it is not a bottleneck
for typical compilation.

Related to Qiskit#10160
@mtreinish mtreinish added performance mod: transpiler Issues and PRs related to Transpiler labels Aug 8, 2023
@mtreinish mtreinish requested a review from a team as a code owner August 8, 2023 13:41
@qiskit-bot
Copy link
Collaborator

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

The previous values used for the preset pass managers were overly
aggressive. It failed CI because the extra overhead for running sabre
made the tests too slow to finish and we hit the job timeout of 60min.
This commit reduces the heuristic effort dials to be only 2x of the
previous values instead of 5x like in the previous commit. Hopefully
this will strike a better balance between runtime and quality of results
on, especially on small environments like the CI worker nodes.
@coveralls
Copy link

coveralls commented Aug 10, 2023

Pull Request Test Coverage Report for Build 5836763974

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.003%) to 87.259%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 91.65%
crates/qasm2/src/parse.rs 6 97.6%
Totals Coverage Status
Change from base Build 5836645682: 0.003%
Covered Lines: 74289
Relevant Lines: 85136

💛 - Coveralls

@mtreinish mtreinish closed this Sep 18, 2023
@mtreinish
Copy link
Member Author

I'm closing this because it causes tests to fail because it makes the transpiler too slow for CI. Drastically increasing the heuristic effort doesn't seem to be the direction we need to take to solve deeper issues in the heuristic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: transpiler Issues and PRs related to Transpiler performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants