-
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!: Use CX count as default cost function #134
Conversation
I've implemented |
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.
👍
src/rewrite/strategy.rs
Outdated
pub fn exhaustive_cx() -> ExhaustiveRewriteStrategy<fn(&OpType) -> bool> { | ||
ExhaustiveRewriteStrategy::with_predicate(is_cx) | ||
} | ||
|
||
/// Exhaustive rewrite strategy with CX count cost function and provided gamma. | ||
pub fn exhaustive_cx_with_gamma(gamma: f64) -> ExhaustiveRewriteStrategy<fn(&OpType) -> bool> { | ||
ExhaustiveRewriteStrategy::new(gamma, is_cx) | ||
} |
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.
these would be nicer as associated functions of ExhaustiveRewriteStrategy
impl ExhaustiveRewriteStrategy<fn(&OpType) -> bool> {
pub fn exhaustive_cx() -> Self {
Self::with_predicate(is_cx)
}
}
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.
Yes you are right, I did the same with default_with_eccs_json_file
in TasoOptimiser
.
I realised testing #133 that
ExhaustiveRewriteStrategy
had its cost function (total gate count) hard coded. I changed that and used the opportunity to use CX count as the default cost function in bothExhaustiveRewriteStrategy
and TASO.