-
Notifications
You must be signed in to change notification settings - Fork 704
Stop promoting numpy to autograd in molecular_hamiltonian #8410
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: v0.43.0-rc0
Are you sure you want to change the base?
Conversation
Automatic update of stable requirement files to snapshot valid python environments. Because bots are not able to trigger CI on their own, please do so by pushing an empty commit to this branch using the following command: ``` git commit --allow-empty -m 'trigger ci' ``` Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Mudit Pandey <[email protected]>
Hello. You may have forgotten to update the changelog!
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v0.43.0-rc0 #8410 +/- ##
==============================================
Coverage ? 99.42%
==============================================
Files ? 584
Lines ? 61242
Branches ? 0
==============================================
Hits ? 60891
Misses ? 351
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pennylane/qchem/hamiltonian.py
Outdated
interface_args = [{"like": "autograd", "requires_grad": requires_grad}, {"like": "jax"}][ | ||
use_jax | ||
] | ||
interface = qml.math.get_deep_interface((coordinates, alpha, coeff)) |
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.
Is this assuming that all 3 input args have the same type? e.g., what happens if coordinates is np array but one of the others is jnp array?
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.
actually, good catch. I had been thinking get_deep_interface
was just get_interface
but on leaves. But forgot that is just takes the first interface it finds. Updated.
tests/qchem/test_hamiltonians.py
Outdated
def test_molecular_hamiltonian_numpy_data(): | ||
"""Test that if numpy data is used with molecular hamiltonian, that numpy data is outputted.""" | ||
symbols = ["H", "H"] | ||
coordinates = np.array([0.0, 0.0, -0.6614, 0.0, 0.0, 0.6614]) |
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 please also use the more common format as well:
coordinates = np.array([[0.0, 0.0, -0.6614], [0.0, 0.0, 0.6614]])
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.
using the 2D array instead now.
pennylane/qchem/hamiltonian.py
Outdated
interface = qml.math.get_deep_interface((coordinates, alpha, coeff)) | ||
if interface == "autograd": | ||
interface_args = {"like": "autograd", "requires_grad": requires_grad} | ||
elif interface in {"numpy", "jax"}: | ||
interface_args = {"like": interface} | ||
else: | ||
raise ValueError(f"unsupported interface {interface} for molecular_hamiltonian") |
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.
Curious how this fixes the numpy to autograd promoting issue?
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.
Before, if it wasn't jax, then it was autograd. Now, if it's numpy, then it stays numpy.
Should this be targetting v0.43.0-rc0? |
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.
You might have to rebase this branch locally, as it's showing changes from master
.
Context:
Noticed warnings in the VQE spin sectors demo about having both autograd and jax data. But we shouldn't have ever been dealing with autograd data, so there shouldn't have been a warning. The problem was that
molecular_hamiltonian
was promoting numpy data to autograd.Description of the Change:
Doesn't promote data to autograd when it is numpy data.
Raising an error if the data in an unsupported type.
Benefits:
Fewer strange warnings in user workflows.
Possible Drawbacks:
Related GitHub Issues:
[sc-100883]