Skip to content

Conversation

@multiphaseCFD
Copy link
Member

@multiphaseCFD multiphaseCFD commented Mar 20, 2024

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 the
    change, 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:

Switch static LAPACK linking to dynamic loading to LAPACK/OpenBLAS.

Description of the Change:

Deprecated static linking against lapack-dev. Now we load openblas libs deployed by scipy for Hermitian decomposition at runtime.

There are two different approaches for scipy.libs search depending on how users use/install lightning

  • Build from source for C++: The C++ backend check the SCIPY_LIBS_PATH macro variable, which is configured at the compile time with the help of CMAKE and its built-in variable ${Python_SITELIB} .
  • Wheels or pip -e . install: if SCIPY_LIBS_PATH doesn't work, we try to use dladdr to get the path\ to\ lightning_qubit_ops.so and we can get \path\to\scipy.libs with path\ to\ lightning_qubit_ops.so\..\..\scipy.libs, given that both scipy and lightning are installed in the same site-packages (which should be always true if the env is set up correctly).

Benefits:

The licence issue of linking against lapack-dev can be avoided.

Possible Drawbacks:

Related GitHub Issues:

AmintorDusko and others added 30 commits March 19, 2024 07:37
Co-authored-by: Amintor Dusko <[email protected]>
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 @multiphaseCFD!

@multiphaseCFD
Copy link
Member Author

Thanks @multiphaseCFD!

Thanks @maliasadi ! According to previous tests, removing the dummy file config.h could lead to failures in wheels building or C++ unit tests. Let's wait and see if there is a failure caused by removing config.h.

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.

🥳

@multiphaseCFD multiphaseCFD removed the do not merge Do not merge PR until this label is removed label Apr 9, 2024
Copy link
Contributor

@dime10 dime10 left a comment

Choose a reason for hiding this comment

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

Great work! :)

@multiphaseCFD multiphaseCFD merged commit 9ec7126 into master Apr 9, 2024
@multiphaseCFD multiphaseCFD deleted the add_dldso branch April 9, 2024 21:49
@AmintorDusko AmintorDusko restored the add_dldso branch April 11, 2024 12:26
@AmintorDusko AmintorDusko deleted the add_dldso branch April 11, 2024 12:27
@maliasadi maliasadi mentioned this pull request Apr 12, 2024
4 tasks
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. ci:use-multi-gpu-runner Enable usage of Multi-GPU runner for this Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants