Expose Sabre heuristic configuration to Python #12171
Merged
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
This exposes the entirety of the configuration of the Sabre heuristic to Python space, making it modifiable without recompilation. This includes some additional configuration options that were not previously easily modifiable, even with recompilation: - the base weight of the "basic" component can be adjusted - the weight of the "basic" and "lookahead" components can be adjusted to _either_ use a constant weight (previously not a thing) or use a weight that scales with the size of the set (previously the only option). - the "decay" component is now entirely separated from the "lookahead" component, so in theory you can now have a decay without a lookahead. This introduces a tracking `Vec` that stores the scores of _all_ the swaps encountered, rather than just dynamically keeping hold of the best swaps. This has a couple of benefits: - with the new dynamic structure for heuristics, this is rather more efficient because each heuristic component can be calculated in separate loops over the swaps, and we don't have to branch within the innermost loop. - it makes it possible in the future to try things like assigning probabilities to each swap and randomly choosing from _all_ of them, not just the best swaps. This is something I've actively wanted to try for quite some time. The default heuristics in the transpiler-pass creators for the `basic`, `lookahead` and `decay` strings are set to represent the same heuristics as before, and this commit is entirely RNG compatible with its predecessor (_technically_ for huge problems there's a possiblity that pulling out some divisions into multiplications by reciprocals will affect the floating-point maths enough to modify the swap selection).
jakelishman
added
Rust
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8728467376Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
ASV highlighted an unexpected change in the benchmarks - the output from Sabre has changed somewhat from #11977, which wasn't intended to have affected the output in any way. I'll mark this as on hold until we've figured that out. |
|
Ok, with #12172 cherry-picked on top of this branch, the same set of benchmarks that I ran in #12172 show RNG compatibility with No need for the "on hold" now. |
henryzou50
reviewed
May 1, 2024
There was a problem hiding this comment.
This PR LGTM and provides a solid foundation for the Heuristic. However, I have a couple of areas I am curious before we merge:
- Efficiency in Adding New Heuristics: As we consider integrating additional heuristics like depth tracking and critical path analysis, these will likely require more extensive information tracking within route.rs. Could you elaborate on potential strategies we might employ to efficiently manage this increased data complexity? Ensuring that these enhancements do not significantly degrade performance will be crucial.
- API Design and Usability: The current setup does a good job of exposing heuristic configurations from Rust to Python. As we continue to enhance this functionality, what are the planned API elements to ensure that users can intuitively interact with the Sabre swap?
|
For the two questions in your review comment: This PR already does a bit to manage the complexity and runtime cost of unused heuristics; there's catches and guards in various places to avoid doing calculations that aren't involved in any given heuristic. Some things about the "efficiency of adding a new heuristic" are going to depend on the heuristic in question; if everything's purely additive and non-interacting, it feels fairly straightforwards to add more on top. The existing For API re-use and the like: that's of course good if we can manage it, but adding new heuristics will likely usually involving doing more speculative calculations based on the current state, so I'd imagine that they usually will involve doing a fair amount of additional coding. The current interfaces are meant to be roughly usable, but if a new heuristic needed a totally different structure, that'd be ok. It's no good trying to predict that before we need it, though - we'd likely make the code harder to understand and slower to run, and there'd be no guarantee that we'd get things right. We can in general expose more to Python space if we want for controllability, though I don't immediately see any places that aren't dynamically controlled. Did you have something in mind? |
Pull Request Test Coverage Report for Build 10162000789Details
💛 - Coveralls |
|
Thanks for your work! I appreciate the clarification on managing the complexity and runtime costs of unused heuristics. I agree that the specifics of integrating new heuristics will largely depend on the nature of the heuristic itself. Regarding the API design and usability, your points about the speculative nature of new heuristics and the potential need for additional coding make a lot of sense. I didn't have anything specific in mind regarding exposing more to Python space. I was more curious about the overall approach and potential strategies for future enhancements. |
henryzou50
approved these changes
Jul 30, 2024
ElePT
added
the
stable backport potential
mergify bot
pushed a commit
that referenced
this pull request
Jul 30, 2024
* Expose Sabre heuristic configuration to Python This exposes the entirety of the configuration of the Sabre heuristic to Python space, making it modifiable without recompilation. This includes some additional configuration options that were not previously easily modifiable, even with recompilation: - the base weight of the "basic" component can be adjusted - the weight of the "basic" and "lookahead" components can be adjusted to _either_ use a constant weight (previously not a thing) or use a weight that scales with the size of the set (previously the only option). - the "decay" component is now entirely separated from the "lookahead" component, so in theory you can now have a decay without a lookahead. This introduces a tracking `Vec` that stores the scores of _all_ the swaps encountered, rather than just dynamically keeping hold of the best swaps. This has a couple of benefits: - with the new dynamic structure for heuristics, this is rather more efficient because each heuristic component can be calculated in separate loops over the swaps, and we don't have to branch within the innermost loop. - it makes it possible in the future to try things like assigning probabilities to each swap and randomly choosing from _all_ of them, not just the best swaps. This is something I've actively wanted to try for quite some time. The default heuristics in the transpiler-pass creators for the `basic`, `lookahead` and `decay` strings are set to represent the same heuristics as before, and this commit is entirely RNG compatible with its predecessor (_technically_ for huge problems there's a possiblity that pulling out some divisions into multiplications by reciprocals will affect the floating-point maths enough to modify the swap selection). * Update for PyO3 0.21 * Increase documentation of heuristic components (cherry picked from commit 43d8372)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 30, 2024
* Expose Sabre heuristic configuration to Python This exposes the entirety of the configuration of the Sabre heuristic to Python space, making it modifiable without recompilation. This includes some additional configuration options that were not previously easily modifiable, even with recompilation: - the base weight of the "basic" component can be adjusted - the weight of the "basic" and "lookahead" components can be adjusted to _either_ use a constant weight (previously not a thing) or use a weight that scales with the size of the set (previously the only option). - the "decay" component is now entirely separated from the "lookahead" component, so in theory you can now have a decay without a lookahead. This introduces a tracking `Vec` that stores the scores of _all_ the swaps encountered, rather than just dynamically keeping hold of the best swaps. This has a couple of benefits: - with the new dynamic structure for heuristics, this is rather more efficient because each heuristic component can be calculated in separate loops over the swaps, and we don't have to branch within the innermost loop. - it makes it possible in the future to try things like assigning probabilities to each swap and randomly choosing from _all_ of them, not just the best swaps. This is something I've actively wanted to try for quite some time. The default heuristics in the transpiler-pass creators for the `basic`, `lookahead` and `decay` strings are set to represent the same heuristics as before, and this commit is entirely RNG compatible with its predecessor (_technically_ for huge problems there's a possiblity that pulling out some divisions into multiplications by reciprocals will affect the floating-point maths enough to modify the swap selection). * Update for PyO3 0.21 * Increase documentation of heuristic components (cherry picked from commit 43d8372) Co-authored-by: Jake Lishman <[email protected]>
Procatv
pushed a commit
to Procatv/qiskit-terra-catherines
that referenced
this pull request
Aug 1, 2024
* Expose Sabre heuristic configuration to Python This exposes the entirety of the configuration of the Sabre heuristic to Python space, making it modifiable without recompilation. This includes some additional configuration options that were not previously easily modifiable, even with recompilation: - the base weight of the "basic" component can be adjusted - the weight of the "basic" and "lookahead" components can be adjusted to _either_ use a constant weight (previously not a thing) or use a weight that scales with the size of the set (previously the only option). - the "decay" component is now entirely separated from the "lookahead" component, so in theory you can now have a decay without a lookahead. This introduces a tracking `Vec` that stores the scores of _all_ the swaps encountered, rather than just dynamically keeping hold of the best swaps. This has a couple of benefits: - with the new dynamic structure for heuristics, this is rather more efficient because each heuristic component can be calculated in separate loops over the swaps, and we don't have to branch within the innermost loop. - it makes it possible in the future to try things like assigning probabilities to each swap and randomly choosing from _all_ of them, not just the best swaps. This is something I've actively wanted to try for quite some time. The default heuristics in the transpiler-pass creators for the `basic`, `lookahead` and `decay` strings are set to represent the same heuristics as before, and this commit is entirely RNG compatible with its predecessor (_technically_ for huge problems there's a possiblity that pulling out some divisions into multiplications by reciprocals will affect the floating-point maths enough to modify the swap selection). * Update for PyO3 0.21 * Increase documentation of heuristic components
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.
Summary
This exposes the entirety of the configuration of the Sabre heuristic to Python space, making it modifiable without recompilation. This includes some additional configuration options that were not previously easily modifiable, even with recompilation:
This introduces a tracking
Vecthat stores the scores of all the swaps encountered, rather than just dynamically keeping hold of the best swaps. This has a couple of benefits:The default heuristics in the transpiler-pass creators for the
basic,lookaheadanddecaystrings are set to represent the same heuristics as before, and this commit is entirely RNG compatible with its predecessor (technically for huge problems there's a possiblity that pulling out some divisions into multiplications by reciprocals will affect the floating-point maths enough to modify the swap selection).Details and comments
I need to run more formal benchmarks on this to verify the claims in the commit message - it's possible I didn't do it sufficiently well earlier, and at any rate I no longer have the data to hand. (edit: come to think of it, I'm not even sure I ran the tests...)
I so far have not documented the newly available API stuff from Rust-space here, because I didn't necessarily want to commit to it as public interface, but did want to make it available for research - certainly I found this very useful in investigating Sabre, even if I haven't had time to turn that research into a full set of tweaked heuristics yet or anything. Happy to discuss whether we should commit to this as public API - there's probably not too much risk.