-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add build-time flag to enable OpenMP in Lightning gate kernels #510
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #510 +/- ##
==========================================
+ Coverage 98.18% 98.67% +0.48%
==========================================
Files 10 169 +159
Lines 1210 24308 +23098
==========================================
+ Hits 1188 23985 +22797
- Misses 22 323 +301 ☔ View full report in Codecov by Sentry. |
...ne_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GateImplementationsLM.hpp
Outdated
Show resolved
Hide resolved
...ne_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GateImplementationsLM.hpp
Outdated
Show resolved
Hide resolved
...ne_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GateImplementationsLM.hpp
Outdated
Show resolved
Hide resolved
...ne_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GateImplementationsLM.hpp
Outdated
Show resolved
Hide resolved
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 @mlxd! LGTM 🎉 I have a few questions regarding the support, but there is nothing major to address from my side.
...ightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/avx_common/AVXGateKernels.hpp
Show resolved
Hide resolved
...ne_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GateImplementationsLM.hpp
Show resolved
Hide resolved
pennylane_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GatePragmas.hpp
Show resolved
Hide resolved
pennylane_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GatePragmas.hpp
Outdated
Show resolved
Hide resolved
pennylane_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GatePragmas.hpp
Show resolved
Hide resolved
…cpu_kernels/GatePragmas.hpp Co-authored-by: Ali Asadi <[email protected]>
...ne_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GateImplementationsLM.hpp
Show resolved
Hide resolved
...ne_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GateImplementationsLM.hpp
Outdated
Show resolved
Hide resolved
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.
LGTM!
pennylane_lightning/core/src/simulators/lightning_qubit/gates/cpu_kernels/GatePragmas.hpp
Outdated
Show resolved
Hide resolved
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.
🪂
…cpu_kernels/GatePragmas.hpp Co-authored-by: Amintor Dusko <[email protected]>
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 @mlxd for the nice work!
Thanks for all of the comments folks! |
Before submitting
Please complete the following checklist when submitting a PR:
All new features must include a unit test.
If you've fixed a bug or added code that should be tested, add a test to the
tests
directory!All new functions and code must be clearly commented and documented.
If you do make documentation changes, make sure that the docs build and
render correctly by running
make docs
.Ensure that the test suite passes, by running
make test
.Add a new entry to the
.github/CHANGELOG.md
file, summarizing thechange, and including a link back to the PR.
Ensure that code is properly formatted by running
make format
.When all the above are checked, delete everything above the dashed
line and fill in the pull request template.
Context: The default threading behaviour for
lightning.qubit
is to assume single threaded execution at the gate level, but allow multi-threaded execution over observables with the adjoint differentiation pipeline. This allows us to always ensure the best (SIMD) gate kernel performance, but limits the performance for situations with deep circuits on large CPUs (high core count, and big caches). This PR adds a compile-time flag, defaulting to off, to allowlightning.qubit
to add OpenMP supports to all kernels (LM, AVX2, and AVX512). this will allow for better performance tuning on HPC systems, while preserving the default behavior during release.Description of the Change: As above. Option enabled through a
LQ_ENABLE_KERNEL_OMP
CMake option in the LQ backend, which is default disabled.Benefits: Allows better performance tuning on HPC systems.
Possible Drawbacks: If on by default, OMP overhead may slow down small circuits. Testing on CI will show no benefit (1 physical core only), so no additional tests added.
Related GitHub Issues: