-
Notifications
You must be signed in to change notification settings - Fork 214
Adds path for the CUDA static library based on CUDA_HOME #837
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
Conversation
@Andy-Jost to trigger CI to run you need to comment "/ok to test". More info here: https://docs.gha-runners.nvidia.com/platform/apps/copy-pr-bot/faqs |
/ok to test 6b178c1 |
This breaks Windows builds because LIBRARY_PATH is specific to Linux. While I could place a guard around the check, it's probably better to close this PR without merging. |
I think it'd be lovely to complete this PR, I stumbled over this many times. Under Windows, we need This is a good way to test for Windows (we use that in
|
I am looking into a solution that works for Windows as well as Linux. |
Probably just update |
37e877d
to
44579bc
Compare
The latest changes eliminate the need to set either LIBRARY_PATH or LIB and should resolve issue #608. Where can I find the source for the installation instructions? I.e., https://nvidia.github.io/cuda-python/cuda-bindings/latest/install.html. Those can be updated now. |
This did the trick. |
d62461d
to
96f3514
Compare
/ok to test 96f3514 |
This comment has been minimized.
This comment has been minimized.
96f3514
to
49ec36d
Compare
Thanks, @Andy-Jost! The diff is clean now. I think to test this properly in the CI, we should delete these two lines cuda-python/.github/workflows/build-wheel.yml Line 160 in b400e34
cuda-python/.github/workflows/build-wheel.yml Line 164 in b400e34
Not sure if I miss any. I hope not 🤔 |
cuda_bindings/setup.py
Outdated
os.path.dirname(sysconfig.get_path("include")), | ||
] + include_path_list | ||
library_dirs = [sysconfig.get_path("platlib"), os.path.join(os.sys.prefix, "lib")] | ||
cudalib_subdir = r"lib\x64" if sys.platform == "win32" else "lib64" |
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.
On the Linux side, one annoying thing is that depending on how CUDA is installed, this could be lib
or lib64
:
$ ls -l /usr/local/cuda-13.0/lib64/libcudart_static.a
-rw-r--r-- 1 root root 1361104 Aug 5 09:32 /usr/local/cuda-13.0/lib64/libcudart_static.a
$ ls -l /usr/local/cuda-13.0/targets/x86_64-linux/lib/libcudart_static.a
-rw-r--r-- 1 root root 1361104 Aug 5 09:32 /usr/local/cuda-13.0/targets/x86_64-linux/lib/libcudart_static.a
We probably should support both.
/ok to test 19aaca9 |
I see a failure in pre-commit.ci due to "ruff format" creating diffs. I think I get the general idea, but I don't see how to get more information (e.g., see the diffs, or run the ruff format locally). I can probably fix the formatting issue by guessing, but in general how can I get the details? |
/ok to test 19aaca9 |
/ok to test 26da853 |
26da853
to
73acddf
Compare
/ok to test 73acddf |
|
@copilot please backport this PR to the 12.9.x branch. (Since this PR touched some files in the .github folder, the backport bot does not work.) |
Git push to origin failed for 12.9.x with exitcode 1 |
Git push to origin failed for 12.9.x with exitcode 1 |
Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>
…bc-85a8c5353e08 Backport PR #837: Add path for CUDA static library based on CUDA_HOME
Both CUDA_HOME (or CUDA_PATH) and LIBRARY_PATH are required to be set in the environment during installation. There was already a check for CUDA_HOME in setup.py, but no check for LIBRARY_PATH. This change adds a check to make sure LIBRARY_PATH is set.
When LIBRARY_PATH is not set, the installation fails at link time and indicates that cudart_static could not be found.