-
Notifications
You must be signed in to change notification settings - Fork 615
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
Torch boundary uses JacobianProductCalculator
#4654
Conversation
Co-authored-by: David Wierichs <[email protected]>
Co-authored-by: David Wierichs <[email protected]>
Co-authored-by: David Wierichs <[email protected]>
Co-authored-by: David Wierichs <[email protected]>
Co-authored-by: David Wierichs <[email protected]>
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 great, thanks @albi3ro! 🎉
Might make sense to rebase to autograd-use-jpc
, or to native-jpc-devices
, again after the auto-rebase?
Otherwise I only had some minor comments, and a changelog entry is needed :)
tests/interfaces/default_qubit_2_integration/test_torch_default_qubit_2.py
Outdated
Show resolved
Hide resolved
tests/interfaces/default_qubit_2_integration/test_torch_default_qubit_2.py
Outdated
Show resolved
Hide resolved
…t_qubit_2.py Co-authored-by: David Wierichs <[email protected]>
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 great, good to go from my side! 🎉
Just two tiny things in the initial torch registration example.
Co-authored-by: David Wierichs <[email protected]>
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.
sweetness. just a few questions about what's going on
tests/interfaces/default_qubit_2_integration/test_torch_default_qubit_2.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Matthew Silverman <[email protected]>
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 PR is a follow on to #4557 .
It updates the torch interface to rely on the
JacobianProductCalculator
class to compute vjp's.Note: I cannot figure out how to replicate the failing test locally. It's finite shots, which always makes me a bit suspect. But passes fine locally 😕