Skip to content
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

Fix demos to use jax that require grads #1226

Open
wants to merge 45 commits into
base: master
Choose a base branch
from

Conversation

austingmhuang
Copy link
Contributor

@austingmhuang austingmhuang commented Oct 1, 2024

Doesn't build due to the fact that this PR is not yet merged. JSON files need to be updated as well.

Summary:
Fixes remaining demos that checked for requires_grad. Won't build for now due to missing a fix in this PR

[sc-69776] [sc-69778]

Copy link

github-actions bot commented Oct 1, 2024

👋 Hey, looks like you've updated some demos!

🐘 Don't forget to update the dateOfLastModification in the associated metadata files so your changes are reflected in Glass Onion (search and recommendations).

Please hide this comment once the field(s) are updated. Thanks!

austingmhuang and others added 13 commits October 1, 2024 17:26
[sc-73935]

* Remove deprecated code from demos.
* Update various `from pennylane import numpy as np` to `import numpy as
np`.
**Summary:**
Fixes a merge conflict between `master` and `dev` introduced by #1232
(due to divergent `dateOfLastModification`).

To reproduce this PR:
1. Run `git checkout dev`.
2. Run `git checkout -b merge-master-into-dev`.
3. Run `git merge master`.
4. Accept all incoming changes for merge conflicts on
`dateOfLastModification`.

**Relevant GHA Workflow Runs:**
* https://github.com/PennyLaneAI/qml/actions/runs/11241666164
* https://github.com/PennyLaneAI/qml/actions/runs/11253088983

---------

Co-authored-by: David Wierichs <davidwierichs@gmail.com>
Co-authored-by: Korbinian Kottmann <43949391+Qottmann@users.noreply.github.com>
Co-authored-by: Ivana Kurečić <ivana@xanadu.ai>
- Branched off dev
- Merged Master in via `--allow-unrelated-histories`

Co-authored-by: Jack Brown <jack.brown486@gmail.com>
@austingmhuang austingmhuang marked this pull request as ready for review October 15, 2024 20:46
@austingmhuang austingmhuang changed the title [WIP] Fix demos to use jax that require grads Fix demos to use jax that require grads Oct 15, 2024
Comment on lines +197 to +199
# In this example, we use the ``default.qubit`` simulator:
num_wires = 6
dev = qml.device("lightning.qubit", wires=num_wires)
dev = qml.device("default.qubit", wires=num_wires)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason lightning.qubit doesn't work with JAX here. Not sure why but I had to change to using default to get it working.

Steps used for this merge:

1. Create new branch from `dev`
2. Merge `master` into that branch
3. Open PR

---------

Co-authored-by: Jack Brown <jack.brown486@gmail.com>
Co-authored-by: Ivana Kurečić <ivana@xanadu.ai>
Copy link
Contributor

@soranjh soranjh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @austingmhuang. before doing a full review, could you please make sure that this PR targets dev instead of master? Also, if there is any demo that does not require features in dev, their upgrade should be done in this PR.

Comment on lines 69 to 70
from jax import numpy as jnp
import jax
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just use vanilla numpy in this demo.

Comment on lines 204 to +209
from pennylane import qchem
from pennylane import numpy as np
from jax import numpy as jnp

symbols = ['H', 'H']
geometry = np.array([[0.0, 0.0, -0.69434785],
[0.0, 0.0, 0.69434785]], requires_grad = False)
geometry = jnp.array([[0.0, 0.0, -0.69434785],
[0.0, 0.0, 0.69434785]])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This demo is already updated in another PR, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's not updated in that PR. Well that PR had a one line change in this demo by accident.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This demo doesn't work without the requires_grad changes because it calls fermionic_hamiltonian directly

from pennylane import numpy as np
import numpy as np
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update this demo in this PR as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it cannot be done. This demo calls electronic_integrals() directly, which checks for requires_grad, leading to an error.

@austingmhuang austingmhuang changed the base branch from master to dev October 23, 2024 21:19
austingmhuang and others added 4 commits October 23, 2024 17:19
Steps used:
* Checkout new branch from `dev`
* Merge `master` using `git merge master`
* Publish branch and open PR

---------

Co-authored-by: Jack Brown <jack.brown486@gmail.com>
Co-authored-by: Ivana Kurečić <ivana@xanadu.ai>
Co-authored-by: bellekaplan <167111433+bellekaplan@users.noreply.github.com>
@obliviateandsurrender obliviateandsurrender changed the base branch from dev to master October 28, 2024 21:09
Copy link

Thank you for opening this pull request.

You can find the built site at this link.

Deployment Info:

  • Pull Request ID: 1226
  • Deployment SHA: 40e1a417fbcc4f421f06d5b46bc9935e9622310c
    (The Deployment SHA refers to the latest commit hash the docs were built from)

Note: It may take several minutes for updates to this pull request to be reflected on the deployed site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants