-
Notifications
You must be signed in to change notification settings - Fork 50
Enable OpenMP pragmas for gate kernels on Linux Wheels #1133
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
base: master
Are you sure you want to change the base?
Conversation
|
Hello. You may have forgotten to update the changelog!
|
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #1133 +/- ##
==========================================
- Coverage 95.84% 89.26% -6.59%
==========================================
Files 243 187 -56
Lines 40695 28938 -11757
==========================================
- Hits 39005 25831 -13174
- Misses 1690 3107 +1417
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/benchmark |
1 similar comment
|
/benchmark |
|
/benchmark |
|
/benchmark |
PennyLane BenchmarksBenchmarks Report for lightning.qubitWeighted Average ScoresDirect execution (without Catalyst)
Aggregated ResultsTime score
Memory score
Complexity scoreNo complexity data available. Detailed Per-Workflow Results
Direct execution (without Catalyst)
System InformationHardware and Software Specifications
Report generated on Thu, 16 Oct 2025 at 01:32:38 (UTC) |
josephleekl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems to work well! Some suggestions for things to update:
- docs to mention this is on by default, and can be toggled on/off with
OMP_NUM_THREADS, and might be a good idea to set to 1 if using e.g. concurrency - update changelog
- in `.github/workflows/tests_lqcpu_python.yml we set it to on, with this change, we don't need to explicitly do that anymore?
|
@josephleekl Now, it's ready to review ⚡ |
|
One question. Doing the OMP ON as default, will not interfere with the |
josephleekl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @maliasadi , LGTM! 🚀
LuisAlfredoNu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @maliasadi
🐊
Co-authored-by: Joseph Lee <[email protected]>
mlxd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just adding the red to prevent this from accidentally merging.
We need to do more intensive eval, including across the python package ecosystem and OSs. Will do a proper review next week
mlxd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one @maliasadi
Just a few queries/suggestions, but will be happy to approve pending your response.
Co-authored-by: Lee James O'Riordan <[email protected]>
For context, the default threading behavior of lightning.qubit assumes single-threaded execution at the gate level, while allowing multi-threaded execution over observables when using the adjoint differentiation pipeline. This design ensures optimal (SIMD) gate kernel performance but can limit performance for deep circuits on large CPUs with many cores and large caches.
The flag LQ_ENABLE_KERNEL_OMP enables OpenMP support across all kernel types (LM, AVX2, and AVX512) which allows for better performance tuning on HPC systems while preserving the default behavior for standard releases. This PR makes this default on LQ wheels and from-src installations.
[latest/latest]
[sc-101572]
[sc-104235]