-
Notifications
You must be signed in to change notification settings - Fork 617
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
Improve support for Torch and Jax with dynamic_one_shot
#5672
Conversation
Co-authored-by: Christina Lee <[email protected]>
[sc-59938] |
Hello. You may have forgotten to update the changelog!
|
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.
Thanks @mudit2812 . Just a couple suggestions and a kudo ;) I'm ready to approved afterwards.
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.
Looks good to me, thanks @mudit2812 .
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.
Thanks @mudit2812 , this is a neat extension of MCM support! :)
Had three small suggestions, a question for understanding and would like to iterate Vincent's kudo!
I will keep iterating with changing the number of shots/seed until the tests pass, but otherwise this should be good to go. |
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.
Thanks @mudit2812 !
dynamic_one_shot
dynamic_one_shot
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5672 +/- ##
==========================================
- Coverage 99.68% 99.67% -0.01%
==========================================
Files 416 416
Lines 39105 38815 -290
==========================================
- Hits 38981 38690 -291
- Misses 124 125 +1 ☔ View full report in Codecov by Sentry. |
Context:
Opened in favour of #5630. Bug fix for #5442. This PR updates
dynamic_one_shot
so that it has better compatibility with thetorch
andjax
interfaces.Description of the Change:
array.astype()
toqml.math.cast
in theapply_operation
dispatch forMidMeasureMP
.qml.math
indynamic_one_shot
.qml.counts
, cast results to ints before converting to strings for lists of MCM values and floats for single MCM values. This is needed because jax arrays are not hashable, and the hash of torch tensors seems to be independent of the value(s) stored inside it. Thus, neither can be used as keys for dictionaries.Benefits:
Better interface support with
dynamic_one_shot
.Possible Drawbacks:
Related GitHub Issues: