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

Merge lightning.gpu and lightning.tensor test CIs and add control by a label #911

Merged
merged 27 commits into from
Sep 19, 2024

Conversation

AmintorDusko
Copy link
Contributor

@AmintorDusko AmintorDusko commented Sep 17, 2024

Context: Merge lightning.gpu and lightning.tensor test CIs and add control by a label

Description of the Change: Merge lightning.gpu and lightning.tensor GPU tests in single Python and C++ CIs controlled by the ci:use-gpu-runner label.

Benefits: Efficient CIs, faster queue, and code reduction.

Possible Drawbacks:

[sc-73818]

Related GitHub Issues:

@AmintorDusko AmintorDusko added the ci:use-gpu-runner Enable usage of GPU runner for this Pull Request label Sep 17, 2024
@AmintorDusko AmintorDusko marked this pull request as draft September 17, 2024 17:35
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 urgent Mark a pull request as high priority label Sep 18, 2024
@AmintorDusko AmintorDusko removed the ci:use-gpu-runner Enable usage of GPU runner for this Pull Request label Sep 18, 2024
@AmintorDusko AmintorDusko marked this pull request as ready for review September 18, 2024 14:43
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.32%. Comparing base (eb709d5) to head (0a2ca07).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #911      +/-   ##
==========================================
+ Coverage   96.22%   96.32%   +0.10%     
==========================================
  Files         214       94     -120     
  Lines       29804    11793   -18011     
==========================================
- Hits        28679    11360   -17319     
+ Misses       1125      433     -692     

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

@AmintorDusko
Copy link
Contributor Author

AmintorDusko commented Sep 18, 2024

Last commit (1b264c4) shows an example of GPU runners not being activated without the ci:use-gpu-runner label.
image

@AmintorDusko AmintorDusko added the ci:use-gpu-runner Enable usage of GPU runner for this Pull Request label Sep 18, 2024
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.

Nice work @AmintorDusko . Just one question: is there a plan to move lightning.kokkos-cuda tests to tests_gpu workflows?

@AmintorDusko
Copy link
Contributor Author

Nice work @AmintorDusko . Just one question: is there a plan to move lightning.kokkos-cuda tests to tests_gpu workflows?

Maybe in the future. This one requires more time.

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.

Looks good to me! Thanks @AmintorDusko for the nice work.

Copy link
Member

@maliasadi maliasadi 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!

.github/workflows/tests_gpu_cpp.yml Outdated Show resolved Hide resolved
.github/workflows/tests_gpu_cpp.yml Show resolved Hide resolved
.github/workflows/tests_gpu_python.yml Outdated Show resolved Hide resolved
.github/workflows/tests_gpu_python.yml Outdated Show resolved Hide resolved
.github/CHANGELOG.md Show resolved Hide resolved
.github/workflows/tests_gpu_cpp.yml Outdated Show resolved Hide resolved
.github/workflows/tests_gpu_cpp.yml Outdated Show resolved Hide resolved
.github/workflows/tests_gpu_python.yml Outdated Show resolved Hide resolved
.github/workflows/tests_gpu_python.yml Outdated Show resolved Hide resolved
.github/workflows/tests_gpu_python.yml Outdated Show resolved Hide resolved
@AmintorDusko
Copy link
Contributor Author

Hi @maliasadi, I updated the workflows as discussed offline and following your suggestions.

Copy link
Member

@maliasadi maliasadi 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! Just a few questions to ensure the compatibility of the support with pennylane-plugin-matrix and on push-to-master.. happy to approve soon 🙂

.github/workflows/tests_gpu_cpp.yml Outdated Show resolved Hide resolved
.github/workflows/tests_gpu_python.yml Show resolved Hide resolved
.github/workflows/tests_gpu_python.yml Outdated Show resolved Hide resolved
Copy link
Member

@maliasadi maliasadi 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! LGTM 🎉

@AmintorDusko AmintorDusko merged commit 1acb133 into master Sep 19, 2024
62 checks passed
@AmintorDusko AmintorDusko deleted the update/GPU-CIs branch September 19, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:use-gpu-runner Enable usage of GPU runner for this Pull Request urgent Mark a pull request as high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants