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 PennyLane LightningKokkos and LightningQubits tests for stable/stable #734

Merged
merged 13 commits into from
May 21, 2024

Conversation

AmintorDusko
Copy link
Contributor

@AmintorDusko AmintorDusko commented May 17, 2024

Context: PennyLane LightningKokkos and LightningQubits tests for stable/stable configuration are currently failing.
This PR looks to solve these problems.

Description of the Change:

The LightningKokkos and LightningQubits stable configurations (v0.36.0) were wrongly being tested with the most recent suite of tests.

PR #725 changed the default Kokkos version used by the LKokkos device. Currently, we build all wheels from scratch in our Plugin Test Matrix workflows, the sum of these two factors led me to expand our Kokkos caching functionality to build and cache any number of Kokkos versions. This is now configurable by an input variable kokkos_version.

I brought the change proposed by @vincentmr to this PR so we can have everything in a single place, to see if it is possible to merge it sooner than later.

Benefits: Tests are now right.

[sc-63561]

Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.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.

@AmintorDusko AmintorDusko added the ci:build_wheels Activate wheel building. label May 17, 2024
Copy link

codecov bot commented May 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.29%. Comparing base (6e6e6ad) to head (7ee2e29).
Report is 78 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (6e6e6ad) and HEAD (7ee2e29). Click for more details.

HEAD has 44 uploads less than BASE
Flag BASE (6e6e6ad) HEAD (7ee2e29)
49 5
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #734       +/-   ##
===========================================
- Coverage   87.20%   57.29%   -29.91%     
===========================================
  Files          83       16       -67     
  Lines       11875     1829    -10046     
===========================================
- Hits        10355     1048     -9307     
+ Misses       1520      781      -739     

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

@AmintorDusko AmintorDusko marked this pull request as ready for review May 17, 2024 20:01
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Thanks @AmintorDusko
Quick round of questions

.github/workflows/tests_lkcpu_python.yml Show resolved Hide resolved
.github/workflows/tests_lkcpu_python.yml Outdated Show resolved Hide resolved
.github/workflows/tests_lqcpu_python.yml Show resolved Hide resolved
@AmintorDusko AmintorDusko requested a review from mlxd May 17, 2024 20:14
@AmintorDusko AmintorDusko changed the title Check/Lkokkos stable stable Fix PennyLane LightningKokkos and LightningQubits tests for stable/stable May 21, 2024
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Thanks @AmintorDusko

Copy link
Member

@multiphaseCFD multiphaseCFD left a comment

Choose a reason for hiding this comment

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

Thanks @AmintorDusko ! Looks good to me!

@AmintorDusko AmintorDusko merged commit d757091 into master May 21, 2024
82 of 83 checks passed
@AmintorDusko AmintorDusko deleted the check/LKokkos_stable_stable branch May 21, 2024 18:29
multiphaseCFD pushed a commit that referenced this pull request May 22, 2024
…able (#734)

* comment pennylane master out

* make Kokkos version configurable; checkout stable tag

* re-trigger CIs

* uncomment pennylane master

* fix wrong condition

* update version

* fix for lqpu #732

* update lkcuda workflow and leave some notes to remember to change it back later

* update stable branch to latest_release

* checkout latest_release on LQubit tests [skip ci]

* Trigger CIs

* update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:build_wheels Activate wheel building.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants