-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: Improve TASO cost function and rewrite strategies #154
Conversation
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.
Awesome.
I think this'll let me remove the hard-coded node cost function in #149 that's used for splitting the circuit.
src/rewrite/strategy.rs
Outdated
/// | ||
/// The minor cost to break ties between equal CX counts is the number of | ||
/// quantum gates. | ||
pub type NonIncreasingCXCountStrategy = |
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.
The name is a bit detached from the actual definition. Any non-inc strategy with static cost functions will type with this alias.
The default_cx
call is the one that introduces the actual CX semantics.
Do you need the alias?
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.
No you're right. I'll remove it.
src/rewrite/strategy.rs
Outdated
/// The cost function must return a value of type `Self::OpCost`. All op costs | ||
/// are summed up to obtain a total cost that is then compared using the | ||
/// threshold function. | ||
pub trait ExhaustiveThresholdStrategy { |
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.
Move this higher up.
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.
Done!
Sorry, this ended up being much bigger than expected. If you have any suggestions to simplify it, I'll gladly take them.
I have added a rewrite strategy that considers not only two-qubit gate count, but also breaks ties with total gate count. I think this should make the optimisation landscape less flat overall.
I've also taken the opportunity to make the optimisation cost function a data of the rewrite strategy, so that it no longer has to be passed separately to TASO.