-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Allow barriers in overlap circuit inputs #11179
Allow barriers in overlap circuit inputs #11179
Conversation
One or more of the the following people are requested to review this:
|
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 ok to me as a temporary workaround, thanks Caleb. I think a more proper solution will be that we need to unify the "can this operation be used in gates?" logic somewhere, because at the moment the same sort of check is used in various places with quite differing results. For example, circuit_to_gate
will reject Barrier
as well. We can look at that in a follow-up, though, because it might involve some behavioural changes.
Could you also add a test to prevent regression?
yes, will do |
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.
My bad, I should have spotted this in the first review: could you add a "bugfix" release note about this? Other than that (and the extra blank line in the test file that I think black will complain about), this looks fine to merge to me as a bugfix, with the proviso I had about that ideally we'll unify this logic somewhere at a larger scale. That wouldn't be suitable for backport, though, so this is good.
Pull Request Test Coverage Report for Build 6774835580Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
Yep, was actually going to ask about this and figured you'd tell me whether this warranted one :) |
If you have an inclination about where it should live and which modules could use the new function (does this only affect circuit library?), I can start an issue and try to unify it in a separate PR. |
Issue here |
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.
Cool, thanks Caleb!
* Allow barriers in overlap circuit inputs * Minor fixes plus test with barriers * Check parameter number on barrier test, similar to other non-failing tests. * test order * Add release note (cherry picked from commit 25c46dc)
* Allow barriers in overlap circuit inputs * Minor fixes plus test with barriers * Check parameter number on barrier test, similar to other non-failing tests. * test order * Add release note (cherry picked from commit 25c46dc) Co-authored-by: Caleb Johnson <[email protected]>
Fixes #11177
Summary
This PR allows the inputs to
UnitaryOverlap
to contain barriers.Details and comments
When checking the circuit to ensure all instructions are unitary operations, we should ignore barriers.