-
Notifications
You must be signed in to change notification settings - Fork 31
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
Provide options in the cut-finder API to turn LO gate and wire cut finding off or on, expose min-reached flag. #586
Conversation
gate_lo: bool = OptimizationSettings().gate_lo | ||
wire_lo: bool = OptimizationSettings().wire_lo |
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.
One might want to investigate what happens when one sets both gate_lo
and gate_lo
to False
. Ideally this will result in an informative error message (right?).
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, for sure.
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.
Also, I want to acknowledge somewhere (i.e., in this comment) that we'll probably change the name of gate_lo
eventually. since I doubt it makes any sense to implement LOCC gate cutting now that https://arxiv.org/abs/2312.11638 is out. I cannot think of a better name right this moment. The proper names will depend how far we get implementing parallel and black box cutting from that paper.
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.
I have gone ahead and added some of the updates that we discussed (7fcfcb1).
This adds a feature, so it (as a whole) will have to wait for 0.8.0. It might be that some of the fixes are worth backporting, though. |
Pull Request Test Coverage Report for Build 9295628099Details
💛 - Coveralls |
Instances of the greedy search (warm start) algorithm still had access to the LOCC wire-cut cost function and did, in certain instances (when only wire cutting was enabled and the greedy search algorithm was being tested out), output costs that were inconsistent with the LO-only scenario that we currently support. This has now been changed to make things consistent and cost functions that track LOCC wire cuts are no longer exposed anywhere. |
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.
This looks good. Only a couple of very minor nitpicks.
I wonder if this could've been two PRs but looks good to me as-is
releasenotes/notes/min-reached-finder-flag-aa6dd9021e165f80.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/new-flags-to-control-cut-finder-search-e499e1ea49abb0bc.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/min-reached-finder-flag-aa6dd9021e165f80.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/new-flags-to-control-cut-finder-search-e499e1ea49abb0bc.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Jim Garrison <[email protected]>
Co-authored-by: Jim Garrison <[email protected]>
Co-authored-by: Jim Garrison <[email protected]>
Co-authored-by: Jim Garrison <[email protected]>
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.
Thank you -- I am happy with this. 🚀
Add fields
gate_lo
andwire_lo
toOptimizationParameters
incutting.automated_cut_finding.py
to allow the user to turn LO gate and wire cutting off or on. The default is to have them both on.To do:
CutBothWires
action. While running the cut-finder on some circuits withgate_lo
set toFalse
(so only wire cut finding enabled), this action was implemented and we realized that there were some new bugs that were not covered by the existing tests.CutBothWires
instances.