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

Bump rng_salt version to v0.40.0 #6854

Merged
merged 43 commits into from
Feb 28, 2025
Merged

Bump rng_salt version to v0.40.0 #6854

merged 43 commits into from
Feb 28, 2025

Conversation

andrijapau
Copy link
Contributor

@andrijapau andrijapau commented Jan 17, 2025

Context:

With v0.40.0 of Pennylane out, we are safe to bump the rng_salt version.

Benefits:

This alters the RNG salt of our test suite, ensuring that tests from last release weren't passing due to some magic salt.

Description of the change:

This involved modifying some tests but increasing their tolerances. This was discussed with the core team to ensure we aren't accidently hiding any problems.

[sc-82812]

@andrijapau andrijapau requested a review from astralcai January 17, 2025 20:07
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.

@andrijapau andrijapau changed the title Bump rng_salt to v0.40.0 Bump rng_salt version to v0.40.0 Jan 17, 2025
@astralcai
Copy link
Contributor

Could you also do a search of all local_salt that's currently in the test suite and remove the ones that are not necessary for the tests to pass anymore?

@andrijapau
Copy link
Contributor Author

@astralcai, I removed the local_salt that don't need to be there. However, lots of new tests are failing as you can see from the CI.

@astralcai
Copy link
Contributor

@astralcai, I removed the local_salt that don't need to be there. However, lots of new tests are failing as you can see from the CI.

I guess now is a good time to briefly investigate if these tests are failing because of an unfortunately bad seed or if they've been passing because of a magically good seed. If it's the former, you can add a local salt to make them pass.

@andrijapau andrijapau added the WIP 🚧 Work-in-progress label Jan 27, 2025
Copy link

codecov bot commented Jan 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (9b45754) to head (672fba1).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6854   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files         484      484           
  Lines       46305    46305           
=======================================
  Hits        46122    46122           
  Misses        183      183           

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

@JerryChen97
Copy link
Contributor

Seems this PR scope extended a bit to proper adjustment to those stochastic tests right?

@JerryChen97
Copy link
Contributor

Seems this PR scope extended a bit to proper adjustment to those stochastic tests right?

Then probably we could also get rid of some unnecessary numeric checks here? For example, https://github.com/PennyLaneAI/pennylane/blob/61dbc7145cb9b0883e3dff817399c983db178279/tests/ops/functions/test_map_wires.py, in this test_map_wires.py no numeric checks should be needed. This also occasionally comes up in stochastic fails.

@andrijapau andrijapau added the do not merge ⚠️ Do not merge the pull request until this label is removed label Jan 30, 2025
Copy link
Contributor Author

@andrijapau andrijapau left a comment

Choose a reason for hiding this comment

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

Comments to go through with team.

Edit: Went through them with team on Feb 27 2025.

@andrijapau andrijapau merged commit e5baae2 into master Feb 28, 2025
46 checks passed
@andrijapau andrijapau deleted the fix-rng-salt-version branch February 28, 2025 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge ⚠️ Do not merge the pull request until this label is removed WIP 🚧 Work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants