[path_finder_dev] path_finder Search Priority v2 #604

rwgk · 2025-05-04T06:02:20Z

Continuation of #578

Completely replace cuda_paths.py to achieve the desired Search Priority (see updated README.md).

Further testing is under #610

copy-pr-bot · 2025-05-04T06:02:23Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

…ty (see updated README.md).

rwgk · 2025-05-05T05:04:08Z

/ok to test 2b74022

github-actions · 2025-05-05T05:22:53Z

Doc Preview CI
🚀 View preview at https://nvidia.github.io/cuda-python/pr-preview/pr-604/
https://nvidia.github.io/cuda-python/pr-preview/pr-604/cuda-core/
https://nvidia.github.io/cuda-python/pr-preview/pr-604/cuda-bindings/
Preview will be ready when the GitHub Pages deployment is complete.

rwgk · 2025-05-05T07:20:03Z

/ok to test 27db0a7

copy-pr-bot · 2025-05-05T08:41:42Z

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.

cuda_bindings/cuda/bindings/_path_finder/README.md

cuda_bindings/cuda/bindings/_path_finder/find_nvidia_dynamic_library.py

cuda_bindings/cuda/bindings/_path_finder/load_nvidia_dynamic_library.py

cuda_bindings/cuda/bindings/_path_finder/load_dl_common.py

…ormat, bandit nosec

rwgk · 2025-05-05T18:57:59Z

/ok to test b1a5e9d

https://chatgpt.com/share/681914ce-f274-8008-9e9f-4538716b4ed7

cuda_python/docs/source/release/12.9.0-notes.rst

The previous implementation checked result_queue.empty() before calling get(), which introduces a classic race condition: the queue may become non-empty immediately after the check, resulting in missed results or misleading errors. This patch replaces the empty() check with result_queue.get(timeout=1.0), allowing the parent process to robustly wait for results with a bounded delay. Also switches from ctx.SimpleQueue() to ctx.Queue() for compatibility with timeout-based get(), which SimpleQueue does not support on Python ≤3.12. Note: The race condition was discovered by Gemini 2.5

cuda_bindings/cuda/bindings/_path_finder/run_python_code_safely.py

rwgk · 2025-05-05T21:06:41Z

Tracking the development history of run_python_code_safely.py up to commit 9b474bc:

Perplexity_run_python_code_safely_original_version.pdf
ChatGPT Deep research followed by feeding back Gemini 2.5 findings
Claude_3.7_Sonnet_2025-05-05+131336.pdf — This didn't lead to any changes.
Gemini_2.5_2025-05-05+132203.pdf — Discovered a "classical race condition"

cuda_bindings/cuda/bindings/_path_finder/load_nvidia_dynamic_library.py

cuda_bindings/cuda/bindings/_path_finder/README.md

leofang · 2025-05-06T02:22:38Z

It is pretty awesome that the search priority update also allows us to drop the old Numba code entirely!

…ority_last() the default.

…e informative.

… aid debugging)

…returns an incompatible `handle`. Use win32api.LoadLibraryEx() instead to ensure self-consistency.

leofang

Looking good! Several previous discussions on exec and subprocess/multiprocessing are now moot by removing these unused code entirely.

cuda_bindings/cuda/bindings/_path_finder/load_nvidia_dynamic_library.py

cuda_bindings/cuda/bindings/_path_finder/run_python_code_safely.py

cuda_bindings/cuda/bindings/_path_finder/load_nvidia_dynamic_library.py

cuda_bindings/cuda/bindings/_path_finder/load_dl_common.py

…od. Move run_python_code_safely.py to test/ directory.

rwgk · 2025-05-06T16:07:54Z

This PR has base branch path_finder_dev. — I'm keeping it around because it's easiest to look at for a final review.

PR #613 has base branch main, but against the same branch.

Interesting/surprising: I triggered the CI under #613, but it shows up here, too.

cuda_bindings/cuda/bindings/_path_finder/README.md

cuda_bindings/tests/run_python_code_safely.py

* Use win32api.GetModuleFileName() in abs_path_for_dynamic_library(). With this, load_dl_windows.py consistently uses win32api. ctypes is no longer needed, which eliminates the potential for confusion due to different types of handles. * Address review comment #604 (comment) by at-kkraus14 * Rename function run_python_code_safely() → run_in_spawed_child_process() * Change run_in_spawned_child_process() to accept a callable function instead of a string with Python code. * ChatGPT suggestions * Add rethrow as suggested by ChatGPT * Better names: Worker → ChildProcessWrapper, func → target * ChatGPT suggestions * Add minimal test_spawned_process_runner.py as generated by ChatGPT

WIP (search priority updated in README.md but not in code)

8c9a2de

rwgk mentioned this pull request May 4, 2025

[path_finder_dev] 2025-05-01 version of cuda.bindings.path_finder #578

Merged

Merge branch 'path_finder_dev' into path_finder_search_priority_v2

8479511

rwgk self-assigned this May 4, 2025

rwgk force-pushed the path_finder_search_priority_v2 branch from 6a49659 to 8ddb806 Compare May 5, 2025 03:33

Completely replace cuda_paths.py to achieve the desired Search Priori…

2cf3fa2

…ty (see updated README.md).

rwgk force-pushed the path_finder_search_priority_v2 branch from 8ddb806 to 2cf3fa2 Compare May 5, 2025 04:46

Define IS_WINDOWS = sys.platform == "win32" in supported_libs.py

2b74022

Use os.path.samefile() to resolve issues with doubled backslashes.

27db0a7

rwgk marked this pull request as ready for review May 5, 2025 08:41

rwgk requested review from leofang and kkraus14 May 5, 2025 08:42

kkraus14 reviewed May 5, 2025

View reviewed changes

leofang reviewed May 5, 2025

View reviewed changes

cuda_bindings/cuda/bindings/_path_finder/load_dl_common.py Outdated Show resolved Hide resolved

rwgk added 4 commits May 5, 2025 10:59

Merge branch 'path_finder_dev' into path_finder_search_priority_v2

e0a0143

`load_in_subprocess(): Pass current environment

1f728c0

Add run_python_code_safely.py as generated by perplexity, plus ruff f…

0d23bb6

…ormat, bandit nosec

Replace subprocess.run with run_python_code_safely

b1a5e9d

rwgk added 2 commits May 5, 2025 12:31

Factor out class Worker to fix pickle issue.

8e9c7b1

ChatGPT revisions based on Deep research:

5977b9d

https://chatgpt.com/share/681914ce-f274-8008-9e9f-4538716b4ed7

leofang reviewed May 5, 2025

View reviewed changes

cuda_python/docs/source/release/12.9.0-notes.rst Outdated Show resolved Hide resolved

leofang reviewed May 5, 2025

View reviewed changes

cuda_bindings/cuda/bindings/_path_finder/run_python_code_safely.py Outdated Show resolved Hide resolved

Resolve SIM108

ab00a87

leofang reviewed May 5, 2025

View reviewed changes

cuda_bindings/cuda/bindings/_path_finder/load_nvidia_dynamic_library.py Outdated Show resolved Hide resolved

leofang reviewed May 5, 2025

View reviewed changes

cuda_bindings/cuda/bindings/_path_finder/load_nvidia_dynamic_library.py Outdated Show resolved Hide resolved

leofang reviewed May 5, 2025

View reviewed changes

cuda_bindings/cuda/bindings/_path_finder/README.md Outdated Show resolved Hide resolved

Change to "nppc" as ANCHOR_LIBNAME

2a039d2

rwgk added 8 commits May 5, 2025 20:25

Implement CUDA_PYTHON_CUDA_HOME_PRIORITY first, last, with default first

f978e67

Remove retry_with_anchor_abs_path() and make retry_with_cuda_home_pri…

782fcf6

…ority_last() the default.

Update README.md to reflect new search priority

676ecb2

SUPPORTED_LINUX_SONAMES does not need updates for CTK 12.9.0

73498c0

The only addition to SUPPORTED_WINDOWS_DLLS for CTK 12.9.0 is nvvm70.dll

7661c13

Make OSError in load_dl_windows.py abs_path_for_dynamic_library() mor…

ddea021

…e informative.

run_cuda_bindings_path_finder.py: optionally use args as libnames (to…

55583d9

… aid debugging)

Bug fix in load_dl_windows.py: ctypes.windll.kernel32.LoadLibraryW() …

a576327

…returns an incompatible `handle`. Use win32api.LoadLibraryEx() instead to ensure self-consistency.

leofang reviewed May 6, 2025

View reviewed changes

Remove _find_nvidia_dynamic_library.retry_with_anchor_abs_path() meth…

5fb2d1f

…od. Move run_python_code_safely.py to test/ directory.

rwgk mentioned this pull request May 6, 2025

Merge path_finder_dev into main #613

Merged

leofang approved these changes May 6, 2025

View reviewed changes

rwgk merged commit 16a3a57 into NVIDIA:path_finder_dev May 6, 2025
75 checks passed

rwgk deleted the path_finder_search_priority_v2 branch May 6, 2025 16:53

rwgk restored the path_finder_search_priority_v2 branch May 6, 2025 16:55

kkraus14 reviewed May 6, 2025

View reviewed changes

cuda_bindings/cuda/bindings/_path_finder/README.md Show resolved Hide resolved

cuda_bindings/tests/run_python_code_safely.py Show resolved Hide resolved

cuda_bindings/tests/run_python_code_safely.py Show resolved Hide resolved

rwgk added a commit to rwgk/cuda-python that referenced this pull request May 7, 2025

Address review comment NVIDIA#604 (comment) by at-kkraus14

d539b43

rwgk mentioned this pull request May 7, 2025

Miscellanous path_finder cleanup post release #621

Merged

rwgk changed the title ~~WIP: path_finder Search Priority v2~~ [path_finder_dev] path_finder Search Priority v2 May 8, 2025

[path_finder_dev] path_finder Search Priority v2 #604

[path_finder_dev] path_finder Search Priority v2 #604

Uh oh!

Conversation

rwgk commented May 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot bot commented May 4, 2025

Uh oh!

rwgk commented May 5, 2025

Uh oh!

github-actions bot commented May 5, 2025

Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

rwgk commented May 5, 2025

Uh oh!

copy-pr-bot bot commented May 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rwgk commented May 5, 2025

Uh oh!

Uh oh!

Uh oh!

rwgk commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leofang commented May 6, 2025

Uh oh!

leofang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rwgk commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rwgk commented May 4, 2025 •

edited

Loading

rwgk commented May 5, 2025 •

edited

Loading