-
Notifications
You must be signed in to change notification settings - Fork 627
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
[Return-types #11] Jax interface support shot vectors #3234
Conversation
…eAI/pennylane into return_jax_shots_vector
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.
@rmoyard this is amazing work! 🔥 Requesting changes because I think _shots_copies
needs some improvements as commented, but the changes shouldn't be major. The PR otherwise is solid.
In addition, ideally, I think it would be best to separate the fix/improvement for compatibility with shot vector defs that contain tuples because it's more general than the JAX interface. Having said that, I see that making it stand-alone would require further testing.
I would be happy to help with this separation by adding unit tests, let me know. I think it's important that we separate such changes because otherwise they will be lost in some bigger scope PRs and it will be challenging to track them down.
Thanks for making the shot tuple fix btw!
…eAI/pennylane into return_jax_shots_vector
…eAI/pennylane into return_jax_shots_vector
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.
Hi @rmoyard, looks good! 🎉 Amazing work. 💯
Approving on the condition of:
- Shot vector tests being separated into a file;
- Mixing up the shots def to have the tuple be at the 1. pos (see suggestion).
The rest of the comments are more minor.
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>
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.
Hi @rmoyard, this is looking solid 💯
Just have a few minor questions, but otherwise this looks good on my end.
…ylane into return_jax_shots_vector
Description of the Change:
This PR integrates shot vectors for QNode with JAX and derivates
jacobian
,hessian
.Benefits:
Users can use shot vectors everywhere with Jax QNode.
Example: