-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add single-qubit wire cutting how-to and expand_observables
function
#368
Conversation
I actually quite like the name |
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.
Going to take a look at notebooks, but a few small comments here. Looking great
idx = final_circuit.find_bit(qubit)[0] | ||
except CircuitError as ex: | ||
raise ValueError( | ||
f"The {i}-th qubit of the `original_circuit` cannot be found " |
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.
So the original and final circuits need to be comprised of the same Qubit
instances, is that right?
If that's the case, we could add a sentence here telling users explicitly to build their two circuit using the same qubit instances. Or maybe this sentence is clear enough? What do you think?
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 change in a5ebfb8 is subtle, but I think it helps to clarify this. Since this function is meant to be used with cut_wires
and cut_wires
already maintains this condition, I don't think we need to belabor it IMO.
Notes on the how-to:
|
I know there is value in showing the rest of the workflow is the same. I'll think more about how to make this more "how-to-y" and less "tutorial-y" |
Yeah, the compromise I made was just to put all the "regular" steps in a single section at the end. |
Pull Request Test Coverage Report for Build 5895817405
💛 - Coveralls |
thanks, not sure how I missed this (copy and paste fail) |
I think I've addressed everything that is immediately actionable here. I'm going to merge it, and we can continue the discussion on anything I missed at #370, since it will mean taking a second pass over the how-to. If there are minor tweaks to |
I put the how-to and the new
expand_observables
function together in the same PR because the how-to requires the function. I think it's a decent name, but I'd be open to something better. (We can always choose a new name later, too.) This is the PR promised in #367.