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

Create a workflow to test PennyLane with NumPy 1.26 #6275

Merged
merged 29 commits into from
Sep 20, 2024

Conversation

PietropaoloFrisoni
Copy link
Contributor

@PietropaoloFrisoni PietropaoloFrisoni commented Sep 17, 2024

Context: In #6061, we started using by default NumPy 2 (instead of NumPy 1) on the CI to test every PR targeting the master branch. Some jobs (for example, those requiring tensorflow) still automatically downgrade to NumPy 1.26 since not all interfaces are supported at the current stage.

This PR aims to create another workflow to test some jobs with NumPy 1.26 (those that don't automatically downgrade to NumPy 1 during the day). Such a workflow should run at night to ensure PennyLane is still compatible with NumPy 1.26.

We'll keep this workflow only temporarily (probably a quarter, but this still has to be established). After that, we'll get rid of it.

Finally, It has been verified (see here) that the tests introduced with this PR are green. This has been verified a second time (see here before merging this PR into master).

Description of the Change: As above.

Benefits: We ensure that PennyLane is still compatible with NumPy 1.26.

Possible Drawbacks:

  • This issue was discovered accidentally (and potentially fixed) during this PR. The TestSinglePrecision class in the test_jax_jit_qnode.py file contains tests that change the JAX precision with jax.config.update("jax_enable_x64", False). When running the jax tests using multiple workers, we can face race conditions if some workers change jax configuration, because other workers in other test classes downgrade to a single precision causing numerous failures. To avoid this, we add --dist=loadscope as additional pytest arg for jax tests, grouping tests by module for test functions and by class for test methods.

  • The new workflow is it is to a large extent a (very simplified) copy of interface-unit-tests.yml. This is against the principle of avoiding code duplication whenever possible. Still, this is just a temporary workflow that lasts for a few months at most (and runs only at night), and it may be convenient to disentangle it as much as possible from the usual test interface.

Related GitHub Issues: None.

Related Shortcut Stories: [sc-73744]

@PietropaoloFrisoni PietropaoloFrisoni marked this pull request as ready for review September 17, 2024 18:56
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@PietropaoloFrisoni PietropaoloFrisoni changed the title Create a workflow to test PL with NumPy 1.x Create a workflow to test PennyLane with NumPy 1.x Sep 17, 2024
Copy link

codecov bot commented Sep 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.70%. Comparing base (de54c54) to head (7691a95).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6275   +/-   ##
=======================================
  Coverage   99.70%   99.70%           
=======================================
  Files         444      444           
  Lines       42133    42133           
=======================================
  Hits        42008    42008           
  Misses        125      125           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PietropaoloFrisoni PietropaoloFrisoni changed the title Create a workflow to test PennyLane with NumPy 1.x Create a workflow to test PennyLane with NumPy 1.26 Sep 17, 2024
@PietropaoloFrisoni
Copy link
Contributor Author

@rashidnhm Please ignore the failures related to JAX 0.4.28 and NumPy 1.26.4 (we'll decide what to do with them at a later stage). I asked for your review mostly for the workflow structure. Thanks!

@lillian542 lillian542 self-requested a review September 19, 2024 13:50
Copy link
Contributor

@lillian542 lillian542 left a 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 :)

.github/workflows/numpy_1_tests.yml Outdated Show resolved Hide resolved
.github/workflows/install_deps/action.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@rashidnhm rashidnhm left a comment

Choose a reason for hiding this comment

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

As discussed offline, this pull request does introduce a lot of code duplication in the workflows. However, due to the urgency of it and it's expected lifetime I believe it is okay to move forward with it as it is and we can explore alternate options with less duplications later on

@PietropaoloFrisoni PietropaoloFrisoni merged commit 2aee2e1 into master Sep 20, 2024
37 checks passed
@PietropaoloFrisoni PietropaoloFrisoni deleted the workflow_numpy_1 branch September 20, 2024 12:02
mudit2812 pushed a commit that referenced this pull request Sep 23, 2024
**Context:** In #6061, we started using by default NumPy 2 (instead of
NumPy 1) on the CI to test every PR targeting the master branch. Some
jobs (for example, those requiring `tensorflow`) still automatically
downgrade to NumPy 1.26 since not all interfaces are supported at the
current stage.

This PR aims to create another workflow to test some jobs with NumPy
1.26 (those that don't automatically downgrade to NumPy 1 during the
day). Such a workflow should run at night to ensure PennyLane is still
compatible with NumPy 1.26.

**We'll keep this workflow only temporarily** (probably a quarter, but
this still has to be established). After that, we'll get rid of it.

Finally, It has been verified (see
[here](https://github.com/PennyLaneAI/pennylane/actions/runs/10927439038))
that the tests introduced with this PR are green. This has been verified
a second time (see
[here](https://github.com/PennyLaneAI/pennylane/actions/runs/10946347640)
before merging this PR into master).

**Description of the Change:** As above.

**Benefits:** We ensure that PennyLane is still compatible with NumPy
1.26.

**Possible Drawbacks:**

- This issue was discovered accidentally (and potentially fixed) during
this PR. The `TestSinglePrecision` class in the `test_jax_jit_qnode.py`
file contains tests that change the JAX precision with
`jax.config.update("jax_enable_x64", False)`. When running the jax tests
using multiple workers, we can face race conditions if some workers
change jax configuration, because other workers in other test classes
downgrade to a single precision causing numerous failures. To avoid
this, we add
[--dist=loadscope](https://pytest-xdist.readthedocs.io/en/latest/distribution.html)
as additional pytest arg for jax tests, grouping tests by module for
test functions and by class for test methods.

- The new workflow is it is to a large extent a (very simplified) copy
of `interface-unit-tests.yml`. This is against the principle of avoiding
code duplication whenever possible. Still, this is just a temporary
workflow that lasts for a few months at most (and runs only at night),
and it may be convenient to disentangle it as much as possible from the
usual test interface.

**Related GitHub Issues:** None.

**Related Shortcut Stories:** [sc-73744]
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.

3 participants