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

Build and package Catalyst wheels with OpenMP and ZStd #457

Merged
merged 12 commits into from
Jan 17, 2024

Conversation

maliasadi
Copy link
Member

@maliasadi maliasadi commented Jan 12, 2024

We now build the Lightning simulator suite with enabling OpenMP and LLVM deps and dialects with ZStd on Linux and macOS. For ABI compatibility, we package our wheels with these libraries and check the hidden deps using

  • auditwheel on Linux
  • delocate on macOS

[sc-52635]
[sc-52636]

@maliasadi maliasadi marked this pull request as ready for review January 16, 2024 14:08
@maliasadi maliasadi changed the title Update wheels Build and package Catalyst wheels with OpenMP and ZStd Jan 16, 2024
@maliasadi maliasadi added the CI Pull requests that update CIs label Jan 16, 2024
@josh146
Copy link
Member

josh146 commented Jan 16, 2024

Thanks @maliasadi! rather than myself reviewing, @mlxd might be a better reviewer here (unless there was something specific you wanted to me check)

@josh146 josh146 removed their request for review January 16, 2024 14:20
Copy link
Contributor

@rauletorresc rauletorresc left a comment

Choose a reason for hiding this comment

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

If I understand correctly, we don't ship libomp and zstd together with our package, we only force their installation from the software manager of the underlaying operating system. In this case, Linux systems would probably install the GCC version and MacOS systems the LLVM based one. Am I right?

@maliasadi
Copy link
Member Author

we don't ship libomp and zstd together with our package, we only force their installation from the software manager of the underlaying operating system. In this case, Linux systems would probably install the GCC version and MacOS systems the LLVM based one. Am I right?

@rauletorresc We indeed ship Catalyst packages with both libomp and zstd using auditwheel on Linux (and delocate-wheels on macOS) as the result of this PR. auditwheel (and delocate) repair our wheels with copying external shared libraries dependencies into the wheel. These tools also modify the appropriate RPATH entries to load these libraries at runtime. Packaging wheels with the OMP runtime and zstd can be considered as good practices as it will ease the installation of packages for users (without any needs to install zstd and omp as additional system requirements). This also ensures the ABI compatibility with external libs in the process.

@rauletorresc
Copy link
Contributor

we don't ship libomp and zstd together with our package, we only force their installation from the software manager of the underlaying operating system. In this case, Linux systems would probably install the GCC version and MacOS systems the LLVM based one. Am I right?

@rauletorresc We indeed ship Catalyst packages with both libomp and zstd using auditwheel on Linux (and delocate-wheels on macOS) as the result of this PR. auditwheel (and delocate) repair our wheels with copying external shared libraries dependencies into the wheel. These tools also modify the appropriate RPATH entries to load these libraries at runtime. Packaging wheels with the OMP runtime and zstd can be considered as good practices as it will ease the installation of packages for users (without any needs to install zstd and omp as additional system requirements). This also ensures the ABI compatibility with external libs in the process.

Would this not conflict/alter/break any previously installed versions of those two libraries at the user's end?

@dime10
Copy link
Contributor

dime10 commented Jan 16, 2024

[sc-52635]
[sc-52636]

@dime10
Copy link
Contributor

dime10 commented Jan 16, 2024

Quick question, does the PR turn on OpenMP for lightning.kokkos and turn it off for lightning.qubit? This was the conclusion we reached for sensible defaults talking to Lee.

Note we should also apply those configuration defaults to our make scripts :)

@maliasadi
Copy link
Member Author

Would this not conflict/alter/break any previously installed versions of those two libraries at the user's end?

@rauletorresc good question! it shouldn't conflict or break any previous version as in previous versions of Catalyst we didn't build our wheels with OMP and Zstd. We enabled them in this PR.

@maliasadi
Copy link
Member Author

maliasadi commented Jan 16, 2024

Quick question, does the PR turn on OpenMP for lightning.kokkos and turn it off for lightning.qubit? This was the conclusion we reached for sensible defaults talking to Lee.

It turns on OMP for both lightning simulators. But for lighnting.qubit, this OMP support is mainly for the computation of adjoint-jacobian in parallel. @mlxd added a separate flag to enable OMP for gates PennyLaneAI/pennylane-lightning#510 that is turned off by default. But, if we don't want to use adjoint-jacobian with OMP, then we may want to consider using LQ_ENABLE_KERNEL_OMP=ON instead for the lightning.qubit simulator. I turn them off as we can always use lightning.kokkos for the OMP support. We can revisit this later 👍

Note we should also apply those configuration defaults to our make scripts :)

Sure, will update Makefiles 🙌

runtime/Makefile Outdated Show resolved Hide resolved
runtime/Makefile Show resolved Hide resolved
@maliasadi maliasadi requested a review from dime10 January 17, 2024 00:11
@dime10 dime10 merged commit c559403 into main Jan 17, 2024
21 checks passed
@dime10 dime10 deleted the maa/add-omp-zstd-wheels branch January 17, 2024 14:46
@maliasadi maliasadi restored the maa/add-omp-zstd-wheels branch January 25, 2024 01:05
josh146 pushed a commit that referenced this pull request Jan 29, 2024
We now build the Lightning simulator suite with enabling OpenMP and LLVM
deps and dialects with ZStd on Linux and macOS. For ABI compatibility,
we package our wheels with these libraries and check the hidden deps
using
- `auditwheel` on Linux
- `delocate` on macOS

[sc-52635]
[sc-52636]
@josh146 josh146 mentioned this pull request Jan 29, 2024
josh146 added a commit that referenced this pull request Jan 29, 2024
**Context:** v0.4.1 bugfix PR

**Description of the Change:**

Incorporates the following PRs: #439 #457 #437 #465 #471

---------

Co-authored-by: David Ittah <dime10@users.noreply.github.com>
Co-authored-by: Ali Asadi <10773383+maliasadi@users.noreply.github.com>
Co-authored-by: Romain Moyard <rmoyard@gmail.com>
Co-authored-by: Rashid N H M <95639609+rashidnhm@users.noreply.github.com>
Co-authored-by: erick-xanadu <110487834+erick-xanadu@users.noreply.github.com>
dime10 added a commit that referenced this pull request Feb 14, 2024
On macOS libomp is typically installed via brew, which doesn't make the
package discoverable by default to avoid conflicting with a potential
GCC installation of OpenMP (not sure that really applies on mac but oh
well).

This issue was introduced in #457 which setup the proper CMake flag for
the wheel recipe but not for local builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Pull requests that update CIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants