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

Apply LLVM customizations for Python wheel build #2504

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

bmhowe23
Copy link
Collaborator

@bmhowe23 bmhowe23 commented Jan 12, 2025

This PR fixes an LLVM discrepancy in our Docker images vs our Python wheels. This should fix #1421 and #1799 for our Python wheels. (The Docker images were already correct.)

Platform Are CUDA-Q Python Wheels Sensitive to the LLVM ARM Relocation Overflow? Are CUDA-Q Python Wheels Sensitive to the MLIR Region Simplification Bug?
x86_64 🟢 --> 🟢 (already good) 🔴 --> 🟢
aarch64 🔴 --> 🟢 🔴 --> 🟢

Just to complicate things even more than they already are, our 0.9.*, the Python wheels were already showing correct behavior for the exact test case in #1799. The exact reason is spelled out in the details of #1799. However, the above table describes the theoretical situation that we could've been vulnerable to the region simplification bug in other conditions.

Signed-off-by: Ben Howe <bhowe@nvidia.com>
@bmhowe23 bmhowe23 force-pushed the pr-apply-llvm-patches-for-manylinux branch from 675e685 to 51e7147 Compare January 12, 2025 04:37
Signed-off-by: Ben Howe <bhowe@nvidia.com>
github-actions bot pushed a commit that referenced this pull request Jan 12, 2025
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

Copy link
Collaborator

@bettinaheim bettinaheim left a comment

Choose a reason for hiding this comment

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

Thank you for finding this.

@bmhowe23 bmhowe23 merged commit e246521 into NVIDIA:main Jan 13, 2025
213 checks passed
@bmhowe23 bmhowe23 deleted the pr-apply-llvm-patches-for-manylinux branch January 13, 2025 16:05
github-actions bot pushed a commit that referenced this pull request Jan 13, 2025
bmhowe23 added a commit to NVIDIA/cudaqx that referenced this pull request Jan 21, 2025
## Primary Changes
* Update build_wheels.yaml to test CUDA-QX wheels after building them.
* Update build_wheels.yaml to build CUDA-Q wheels rather than using
released PyPi CUDA-Q wheels. This was necessary because the current
0.9.1 CUDA-Q wheels have an ARM bug that were impacting our ability to
do testing. This was fixed in
NVIDIA/cuda-quantum#2504, which has not been
released yet.
* Fix RPATH of decoder plugin example (thanks for the suggestion,
@melody-ren) to fix a wheel packaging problem where `auditwheel` as
duplicating (aka grafting) the `libcudaq-qec.so` file, causing
`extension_point` registration anomalies and decoder plugin lookup
failures.

## Secondary Changes
* In .github/actions/get-cudaq-version/action.yaml, skip `jq` install if
it is already installed
* Add get-cudaq-wheels action; currently only invoked on manually
commanded build_wheels.yaml workflow but we may decide to invoke this on
a per-PR basis in the future.
* Change retention period of all uploaded artifacts to 14 days
* Update `getCUDAQXLibraryPath` to work for both QEC and Solvers
* Change decoder-plugins path from a build-time constant (which will not
work for wheels) to use `getCUDAQXLibraryPath` instead.
* Change use of shared_ptr to unique_ptr in decoder plugin handling
* Move decoder plugin helper functions into namespaces rather than
global namespace
* Change cudaqx wheels to be depending on cuda-quantum >= 0.9 rather
than ~= 0.9.0 for better futureproofing of our released wheels

Sample run result w/ custom CUDA-Q wheels:
https://github.com/NVIDIA/cudaqx/actions/runs/12855925162
Sample run result w/ released PyPI CUDA-Q wheels:
https://github.com/NVIDIA/cudaqx/actions/runs/12856394702

---------

Signed-off-by: Ben Howe <bhowe@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM aarch64 relocation overflow
2 participants