-
Notifications
You must be signed in to change notification settings - Fork 159
2025-05-01 version of cuda.bindings.path_finder
#578
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
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
/ok to test 17478da |
|
/ok to test 7da74bd |
…ions 12.0, 12.1, 12.2
/ok to test a649e7d |
For completeness: I used these command while working on commit a649e7d:
|
Test results for commit a649e7d:
import sys
def get_info_abs_path(filename):
print_buffer = [filename]
done = set()
for line in open(filename).read().splitlines():
if "Z INFO " in line:
flds = line.split(": abs_path=", 1)
assert len(flds) == 2
abs_path = eval(flds[1]) # eval undoes repr double backslashes
if abs_path not in done:
print_buffer.append(f" {abs_path}")
done.add(abs_path)
return print_buffer
def run(args):
no_info = []
has_info = []
for filename in sorted(args):
print_buffer = get_info_abs_path(filename)
assert print_buffer
if len(print_buffer) == 1:
no_info.append(print_buffer)
else:
has_info.append(print_buffer)
for print_buffer in no_info + has_info:
print("\n".join(print_buffer))
if __name__ == "__main__":
run(args=sys.argv[1:]) |
…d_dl_windows.py with the help of Cursor.
2452fdb
to
b5cef1b
Compare
/ok to test b5cef1b |
… passed), followed by ruff auto-fixes
/ok to test 001a6a2 |
…ll tests passed), followed by ruff auto-fixes" This reverts commit 001a6a2. There were many GitHub Actions jobs that failed (all tests with 12.x): https://github.com/NVIDIA/cuda-python/actions/runs/14677553387 This is not worth spending time debugging. Especially because * Cursor has been unresponsive for at least half an hour: We're having trouble connecting to the model provider. This might be temporary - please try again in a moment. * The refactored code does not seem easier to read.
/ok to test 0cd20d8 |
The versioned names will be found, too, with the code as-is. See
We have
The only advantage would be that we'd pick out e.g. |
👍 I didn't follow this properly. https://github.com/rwgk/cuda-python/blob/aeaf4f02278b62befb0e380e9f6f97a50b848fb3/cuda_bindings/cuda/bindings/_path_finder/find_nvidia_dynamic_library.py#L33-L37 This will be potentially problematic. If there's no I think the only way this could happen today is by installing wheels since there's both Would it maybe make more sense to just looking for the specific version we'd expect based on cuda 11 vs cuda 12? |
It's definitely more predictable, but I'll need to add in determining the CUDA driver version. I'll work on that. To explain why I was hesitating before: Maximize portability Currently this code would work as pure Python code, even without all the rest of cuda.bindings. I believe that determining the CUDA driver version will either introduce a a dependency on cuda.bindings.driver, or if portability is (or becomes at some point) important, we'd need to reimplement a minimalistic version. |
We shouldn't need to determine the CUDA driver version, we just need the major version which we're already capturing in the We just need a map of CUDA major version --> soname per library I think? |
Oh! Thanks, I need to get my head around that. |
…ced by perplexity
There is a catch here, which is what the current
|
…ual inspection of cuda_paths.py. Minor additional edits.
Question, scoped to the Currently I'm looking through If we're now targeting, e.g., "just 11" or "12 or 11", do we want to fully traverse |
(btw I merged #593) |
/ok to test aeaf4f0 |
We want to find the library as if the extension module was normally built and linked against the library we're finding, which means that it would only search for the |
…ind_sub_dirs_all_sitepackages() from find_nvidia_dynamic_library.py
This reverts commit bf9734c.
There are two changes I still need to work on:
Doing that work under this PR would be unwieldy. This PR has so many commits and comments already, I have to click "Load more ..." several times to find the things I'm looking for. I'll merge this PR into the |
cuda.bindings.path_finder
cuda.bindings.path_finder
This work was merged into the
path_finder_dev
branch (see comment below). Follow-on work is under #604.Description
Major milestone for the work tracked under #451
This PR introduces only two public APIs:
cuda.bindings.path_finder.SUPPORTED_LIBNAMES
(currently('nvJitLink', 'nvrtc', 'nvvm')
)cuda.bindings.path_finder.load_nvidia_dynamic_library(libname: str) -> LoadedDL
With:
However, the implementations were actually thoroughly tested (under #558) for all
enumerated under
cuda.bindings._path_finder.supported_libs
(note that this module is private).To make this PR easier to review, the changes to the
nvJitLink
,nvrtc
, andnvvm
bindings are NOT included in this PR. Those changes were also already tested under #558. They will be merged intomain
with two follow-on PRs (one for thenvrtc
bindings, one fornvJitLink
andnvvm
).Thorough testing of all
SUPPORTED_LIBNAMES + PARTIALLY_SUPPORTED_LIBNAMES
requires changes to the GitHub Actions configs, to set up suitable CTK installations. This will also be handled separately in follow-on PRs.Suggested order for reviewing files:
cuda/bindings/_path_finder/supported_libs.py
cuda/bindings/_path_finder/load_nvidia_dynamic_library.py
cuda/bindings/_path_finder/load_dl_common.py
cuda/bindings/_path_finder/load_dl_linux.py
cuda/bindings/_path_finder/load_dl_windows.py
tests/test_path_finder.py
cuda/bindings/_path_finder/find_nvidia_dynamic_library.py
Discussion points:
cuda/bindings/_path_finder/cuda_paths.py
(the original file under numba-cuda does not have one)SUPPORTED_LIBNAMES + PARTIALLY_SUPPORTED_LIBNAMES
as new CTK versions are released