feat(pathfinder): centralize CUDA env var handling, prioritize CUDA_PATH over CUDA_HOME#1801
feat(pathfinder): centralize CUDA env var handling, prioritize CUDA_PATH over CUDA_HOME#1801rwgk wants to merge 24 commits intoNVIDIA:mainfrom
CUDA_PATH over CUDA_HOME#1801Conversation
…d onto main. Adds cuda.pathfinder._utils.env_var_constants with canonical search order, enhances get_cuda_home_or_path() with robust path comparison and caching, and updates documentation across all packages to reflect the new priority. Co-authored-by: Rob Parolin <rparolin@nvidia.com> Made-with: Cursor
|
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. |
|
Decision: drop Both
Both Related commit: 2164c33 |
Drop os.pathsep splitting of CUDA_PATH/CUDA_HOME in both build_hooks.py files. Both functions now delegate to get_cuda_home_or_path() from cuda.pathfinder, returning a single path. See NVIDIA#1801 (comment) Made-with: Cursor
|
Decision: treat empty Rob's original implementation preserved empty strings (
This addresses my review comment on Rob's PR: #1519 (comment) |
See NVIDIA#1801 (comment) for the rationale Made-with: Cursor
Made-with: Cursor
…lti-path test Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Safe: currently an internal-only API (not yet public). Made-with: Cursor
Export get_cuda_path_or_home from cuda.pathfinder.__init__. External consumers now import from cuda.pathfinder directly. Rename constant to _CUDA_PATH_ENV_VARS_ORDERED and remove all public references to it. Made-with: Cursor
…ng (Cursor) Made-with: Cursor
Pathfinder 1.5.0 release notes no longer claim cross-package consistency (that depends on future bindings/core releases). cuda_bindings env var docs now defer to pathfinder release notes for migration guidance. Made-with: Cursor
|
Dump of findings from independent review with Cursor GPT-5.4 Extra High: |
…docs/nv-versions.json before Discovered via independent review from GPT-5.4 Extra High
Aligns the provenance label with the new CUDA_PATH-first priority. The label signals the highest-priority env var name, not necessarily which variable supplied the value. Discovered via independent review from GPT-5.4 Extra High Made-with: Cursor
|
Responses to #1801 (comment)
This is understood: we cannot immediately pin cuda-bindings and cuda-core to cuda-pathfinder>=1.5 because that version is not released yet. We will resolve this chicken-and-egg situation by releasing cuda-pathfinder 1.5.0 (shortly after this PR is merged), then adding the pins before any new cuda-bindings or cuda-core release is made.
It was an oversight that
This was intentional. The rationale discussed before (with Opus 4.6 1M Thinking) previously was: simple is best here: it's OK if there are downstream failures if
Fixed with commit 6d065e9
Yes this was intentional: #1801 (comment) Since this affects the build hooks only, we believe it'd be more distracting than helpful to mention it in the release notes or other documentation. |
|
/ok to test |
The build backends run in an isolated venv created by pyproject-build. Although cuda-pathfinder is listed in build-system.requires and gets installed, the cuda namespace package from backend-path=["."] shadows the installed cuda-pathfinder, making `from cuda.pathfinder import ...` fail with ModuleNotFoundError. This broke all CI wheel builds. Revert _get_cuda_path() to use os.environ.get() directly with CUDA_PATH > CUDA_HOME priority, and remove cuda-pathfinder from build-system.requires (it was not there on main; our PR added it). Made-with: Cursor
|
/ok to test |
|
|
/ok to test |
|
Successful full CI: https://github.com/NVIDIA/cuda-python/actions/runs/23447214012?pr=1801 |
|
Commit 1eff0f5 only changed the release notes, therefore I canceled the CI, to not waste our resources. See URL in previous comment for a link to a successful full run. |
There was a problem hiding this comment.
One blocking regression remains in the new CUDA_PATH flow:
conftest.pyandcuda_core/tests/conftest.pynow only check whether a CUDA env var is set. IfCUDA_PATH/CUDA_HOMEpoints at a stale or incomplete root, we stop skipping and fail later in compilation instead.
I left inline suggestions on the affected lines.
|
|
||
| import pytest | ||
|
|
||
| from cuda.pathfinder import get_cuda_path_or_home |
There was a problem hiding this comment.
To keep the repo-level core Cython gate accurate after the env-var change, this file needs os again if you preserve the include/ existence check below.
| from cuda.pathfinder import get_cuda_path_or_home | |
| import os | |
| import pytest | |
| from cuda.pathfinder import get_cuda_path_or_home |
conftest.py
Outdated
|
|
||
| def pytest_collection_modifyitems(config, items): # noqa: ARG001 | ||
| cuda_home = os.environ.get("CUDA_HOME") | ||
| cuda_home = get_cuda_path_or_home() |
There was a problem hiding this comment.
Blocking: this now treats any non-empty CUDA_PATH/CUDA_HOME as if headers are available. If the env var points at a stale or incomplete root, the core Cython tests stop skipping and fail later in compilation instead. Please keep the include/ existence check here.
| cuda_home = get_cuda_path_or_home() | |
| cuda_root = get_cuda_path_or_home() | |
| cuda_home = cuda_root if cuda_root is not None and os.path.isdir(os.path.join(cuda_root, "include")) else None |
There was a problem hiding this comment.
Done: commit 9c13237
I believe skipping if CUDA_PATH is set, but there is no include/, is likely to mask oversights. My commit introduces a hard, obvious failure in that situation. I believe that'll be more helpful than skipping, which is likely to get overlooked.
| skipif_need_cuda_headers = pytest.mark.skipif( | ||
| not os.path.isdir(os.path.join(os.environ.get("CUDA_PATH", ""), "include")), | ||
| get_cuda_path_or_home() is None, | ||
| reason="need CUDA header", | ||
| ) |
There was a problem hiding this comment.
Same regression here: any non-empty CUDA_PATH/CUDA_HOME now marks headers as available. test_cooperative_launch() includes <cooperative_groups.h>, so a stale toolkit root stops skipping and fails inside Program.compile() instead. Please keep the include-dir check at this marker.
| skipif_need_cuda_headers = pytest.mark.skipif( | |
| not os.path.isdir(os.path.join(os.environ.get("CUDA_PATH", ""), "include")), | |
| get_cuda_path_or_home() is None, | |
| reason="need CUDA header", | |
| ) | |
| skipif_need_cuda_headers = pytest.mark.skipif( | |
| (cuda_root := get_cuda_path_or_home()) is None or not os.path.isdir(os.path.join(cuda_root, "include")), | |
| reason="need CUDA header", | |
| ) |
There was a problem hiding this comment.
This was deliberate (see comment above), but commit 9c13237 resolves this, and your other feedback above, in a safer way:
- We skip if
CUDA_PATH/CUDA_HOMEis not set. - If set, we fail hard if the
include/directory doesn't exist.
(I believe in practice that amounts to "hey, your CUDA_PATH is pointing to the wrong directory"; which will be obvious.)
| @@ -183,20 +183,23 @@ def find_in_conda(ctx: SearchContext) -> FindResult | None: | |||
|
|
|||
|
|
|||
| def find_in_cuda_home(ctx: SearchContext) -> FindResult | None: | |||
There was a problem hiding this comment.
Non-blocking architecture note: after the recent descriptor/search-step refactors, this helper now represents the shared CUDA env-var anchor rather than specifically CUDA_HOME. Renaming this step (and the analogous header step/tests) to something like find_in_cuda_env() would keep the step layer aligned with the centralized get_cuda_path_or_home() helper and the new found_via="CUDA_PATH" provenance.
| def find_in_cuda_home(ctx: SearchContext) -> FindResult | None: | |
| def find_in_cuda_env(ctx: SearchContext) -> FindResult | None: |
There was a problem hiding this comment.
Done: commit 0255083
I decided to simply make this find_in_cuda_path. cuda_env is too ambiguous (no obvious connection to CUDA_PATH or CUDA_HOME).
Thanks for catching this; I overlooked it.
Add a helper that skips tests when no CUDA path is set, but asserts that the include/ subdirectory exists when one is — surfacing stale or incomplete toolkit roots at collection time instead of letting them fail later in compilation. Applied in both the root conftest.py and cuda_core/tests/conftest.py. Made-with: Cursor
Closes #1433
Continuation of #1519 (squash-merged and rebased onto
main; @rparolin as co-author).Summary
Centralizes
CUDA_PATH/CUDA_HOMEhandling incuda-pathfinderand establishes a consistent priority:CUDA_PATHtakes precedence overCUDA_HOMEwhen both are set.cuda.pathfinder.get_cuda_path_or_home()returns the highest-priority CUDA root directory. Warns when both vars are set and differ.CUDA_PATH>CUDA_HOME: All consumers now go through the centralized function. The previous ad-hocCUDA_HOME > CUDA_PATHordering in some files is removed.CUDA_PATH=""no longer shadows a validCUDA_HOME.found_viaprovenance label: Changed from"CUDA_HOME"to"CUDA_PATH"inLoadedDL,LocatedHeaderDir,LocatedStaticLib, andLocatedBitcodeLibto match the new priority.os.pathsep-based splitting inbuild_hooks.pyis removed. — We are unaware of real-world usage. This could be added back later if needed. At this stage we believe the extra complexity isn't warranted.conftest.py,cuda_coretests/examples,cuda_bindingsexamples all useget_cuda_path_or_home().cuda_bindingsenvironment variable docs updated, API docs updated.Build hooks and
cuda.pathfinderat build timeThe
build_hooks.pyfiles incuda_bindingsandcuda_corecurrently useos.environ.get("CUDA_PATH", os.environ.get("CUDA_HOME"))directly instead ofget_cuda_path_or_home(). This is necessary because in PEP 517 isolated build environments,backend-path=["."]causes thecudanamespace package to resolve to only the project'scuda/directory, shadowing the installedcuda-pathfinderin the build-env's site-packages.A proven workaround exists: replace the
_NamespacePathwith a plainlistthat includes the site-packagescuda/directory, allowingimport cuda.pathfinderto succeed. This is demonstrated and tested in PR #1803. Aftercuda-pathfinder >= 1.5is released on PyPI, a follow-up PR will switch the build hooks to useget_cuda_path_or_home().