-
Notifications
You must be signed in to change notification settings - Fork 63
Fix bug in PCPhase bind #2467
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
base: main
Are you sure you want to change the base?
Fix bug in PCPhase bind #2467
Conversation
- temporary workaround was causing errors in benchmarks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2467 +/- ##
==========================================
+ Coverage 96.74% 96.75% +0.01%
==========================================
Files 156 156
Lines 16937 16990 +53
Branches 1648 1653 +5
==========================================
+ Hits 16385 16438 +53
- Misses 402 403 +1
+ Partials 150 149 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JerryChen97
left a comment
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.
Might need sync the submodule so that we dont touch enzyme etc via this pr🤔
JerryChen97
left a comment
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 mind add a minimal test somewhere? Not sure if this is easy to do; curious why Catalyst's tests never caught this obvious bug...
| *wires, | ||
| angle, | ||
| dim, | ||
| *ctrl_wires, |
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.
Could you add tests in frontend/tests/pytest/from_plxpr?
Context: We are trying to see if we can run certain circuits with capture, qjit and decomposition. At the moment, this is limited by an error arising from
jax_primitivesdue to a workaround which has thus far been left in place in the bind function ofPCPhase.Description of the Change: We remove the workaround and match the signature of the
PCPhaseto what is expected injax_primitives.Benefits: We are closer to using
PCPhasein circuits with capture, qjit and decomps.Possible Drawbacks: Not sure yet how this change will propagate.
Related ShortCut Stories: [sc-101673] [sc-101776]