Skip to content

Initial version of cuda.pathfinder._find_nvidia_headers for nvshmem - #661

Merged
rwgk merged 32 commits into
NVIDIA:mainfrom
rwgk:find_nvidia_headers_nvshmem
Sep 5, 2025
Merged

Initial version of cuda.pathfinder._find_nvidia_headers for nvshmem#661
rwgk merged 32 commits into
NVIDIA:mainfrom
rwgk:find_nvidia_headers_nvshmem

Conversation

@rwgk

@rwgkrwgk commented May 28, 2025

Copy link
Copy Markdown
Contributor

Description

Bump cuda-pathfinder version to TODO

Minimal viable product, currently not a public API (by way of using an underscore prefix: _find_nvidia_headers).

To support nvshmem4py.

Commit 673c38c was transferred from PR #864.

@copy-pr-bot

Copy link
Copy Markdown
Contributor

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.

@rwgk

rwgk commented May 28, 2025

Copy link
Copy Markdown
ContributorAuthor

/ok to test

@rwgkrwgk self-assigned this May 28, 2025
@github-actions

This comment has been minimized.

@leofang
leofang self-requested a review May 28, 2025 22:26
@leofangleofang added P0 High priority - Must do! feature New feature or request cuda.bindings Everything related to the cuda.bindings module labels May 28, 2025
@ZzEeKkAa

Copy link
Copy Markdown
Contributor

Just wondering what is the status for this PR?

@leofangleofang added cuda.pathfinder Everything related to the cuda.pathfinder module and removed cuda.bindings Everything related to the cuda.bindings module labels Jul 18, 2025
@rwgk

rwgk commented Jul 18, 2025

Copy link
Copy Markdown
ContributorAuthor

I still need to fix this up, after merging #723 a couple days ago.

@isVoid

Copy link
Copy Markdown
Contributor

Is there some mechanism so that user can override the path returned by the api? Or that's largely a library duty to maintain?

@rwgkrwgk changed the title Initial version of path_finderfind_nvidia_headers.py (Minimal Viable Product)[cuda_pathfinder] Initial version of find_nvidia_headers.py for nvshmem (Minimal Viable Product)Aug 12, 2025
@rwgk

rwgk commented Aug 12, 2025

Copy link
Copy Markdown
ContributorAuthor

/ok to test

@ZzEeKkAa

Copy link
Copy Markdown
Contributor

Will it tolerate CUDA_HOME and CUDA_PATH ?

@rwgk

rwgk commented Aug 12, 2025

Copy link
Copy Markdown
ContributorAuthor

Will it tolerate CUDA_HOME and CUDA_PATH ?

The code added in this PR ignores those completely. I've only tested these situations (copy-pasted from the current cuda_pathfinder/tests/test_find_nvidia_headers.py):

 # pip install nvidia-nvshmem-cu12
# pip install nvidia-nvshmem-cu13
# conda create -y -n nvshmem python=3.12
# conda activate nvshmem
# conda install -y conda-forge::libnvshmem3 conda-forge::libnvshmem-dev
# wget https://developer.download.nvidia.com/compute/cuda/repos/ubuntu2404/x86_64/cuda-keyring_1.1-1_all.deb
# sudo dpkg -i cuda-keyring_1.1-1_all.deb
# sudo apt update
# sudo apt install libnvshmem3-cuda-12 libnvshmem3-dev-cuda-12
# sudo apt install libnvshmem3-cuda-13 libnvshmem3-dev-cuda-13

Offline @leofang wrote that getting headers in general is now the top priority (I just created #832 to track the work). That will bring in CUDA_HOME / CUDA_PATH for headers. The next step will be to get an overview of where the headers live.

Coming back to nvshmem: Can those headers also be installed into CUDA_HOME / CUDA_PATH? (How is that usually done?)

@rwgk
rwgkforce-pushed the find_nvidia_headers_nvshmem branch from b1ceca6 to eb2e78aCompareSeptember 3, 2025 23:31
@rwgk
rwgkforce-pushed the find_nvidia_headers_nvshmem branch from c822c15 to c90c393CompareSeptember 4, 2025 00:30
@rwgk

rwgk commented Sep 4, 2025

Copy link
Copy Markdown
ContributorAuthor

/ok to test

@leofangleofang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have not reviewed tests, just the implementation.

Comment threadcuda_pathfinder/cuda/pathfinder/_headers/find_nvidia_headers.py Outdated
Comment threadcuda_pathfinder/cuda/pathfinder/_utils/conda_env.py Outdated
Comment threadcuda_pathfinder/cuda/pathfinder/_utils/conda_env.py Outdated
Comment threadcuda_pathfinder/cuda/pathfinder/_utils/conda_env.py Outdated
Comment threadcuda_pathfinder/cuda/pathfinder/_utils/conda_env.py Outdated
Comment threadcuda_pathfinder/cuda/pathfinder/_utils/env_vars_for_include.py Outdated
leofang
leofang previously approved these changes Sep 5, 2025
@github-project-automationgithub-project-automationBot moved this from Todo to In Review in CCCLSep 5, 2025
# Installed from a wheel
nvidia_sub_dirs = ("nvidia", "nvshmem", "include")
hdr_dir: str # help mypy
for hdr_dir in find_sub_dirs_all_sitepackages(nvidia_sub_dirs):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should do it in a follow up, but I think we should really move towards using an importlib based resolution method instead of walking the sitepackages ourselves.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought we discussed and agreed walking the paths is acceptable since on the PyPI side they are predictable? Any reason to prefer importlib?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

That's a good point, tx, I created issue #949 to track this.

(I wrote this code before @ZzEeKkAa pointed me to importlib while working on PR #864)

@rwgkrwgkSep 5, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I thought we discussed and agreed walking the paths is acceptable since on the PyPI side they are predictable? Any reason to prefer importlib?

Lines crossed here. Let's review under #949 when we get to it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

864?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Oops, sorry, copy-paste mishap. Corrected (949).

Comment on lines +56 to +57
"site-packages", # pip install
"dist-packages", # apt install

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need to do anything for conda packages here?

@rwgkrwgkSep 5, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This is gated by have_nvidia_nvshmem_package(). ... But I just realized, there is no nvshmem .deb that installs into dist-packages. So I removed dist-packages from here to not implicitly suggest that it exists: commit 2426260

While at it, I added additional test (low-hanging fruits): commit b74a84c

Interactive testing with conda and "all_must_work" set:

$ pytest -ra -s -vv tests/test_find_nvidia_headers.py
========================================================================= test session starts ==========================================================================
platform linux -- Python 3.12.11, pytest-8.4.1, pluggy-1.6.0 -- /home/rgrossekunst/miniforge3/envs/nvshmem/bin/python3.12
cachedir: .pytest_cache
rootdir: /home/rgrossekunst/forked/cuda-python/cuda_pathfinder
configfile: pyproject.toml
collected 2 items
tests/test_find_nvidia_headers.py::test_unknown_libname PASSED
tests/test_find_nvidia_headers.py::test_find_libname_nvshmem PASSED
============================================================================= INFO summary =============================================================================
INFO test_find_libname_nvshmem: hdr_dir='/home/rgrossekunst/miniforge3/envs/nvshmem/include'
========================================================================== 2 passed in 0.01s ===========================================================================

Without wheel or conda ("all_must_work" set):

$ pytest -ra -s -vv tests/test_find_nvidia_headers.py
========================================================================= test session starts ==========================================================================
platform linux -- Python 3.12.3, pytest-8.4.2, pluggy-1.6.0 -- /home/rgrossekunst/JunkVenv/bin/python3
cachedir: .pytest_cache
rootdir: /home/rgrossekunst/forked/cuda-python/cuda_pathfinder
configfile: pyproject.toml
collected 2 items
tests/test_find_nvidia_headers.py::test_unknown_libname PASSED
tests/test_find_nvidia_headers.py::test_find_libname_nvshmem PASSED
============================================================================= INFO summary =============================================================================
INFO test_find_libname_nvshmem: hdr_dir='/usr/include/nvshmem_13'
========================================================================== 2 passed in 0.02s ===========================================================================

I also did negative tests for both, by using bad expected paths, and they fail as expected.

@rwgk

rwgk commented Sep 5, 2025

Copy link
Copy Markdown
ContributorAuthor

/ok to test

@rwgk
rwgk merged commit d64120b into NVIDIA:mainSep 5, 2025
49 checks passed
@github-project-automationgithub-project-automationBot moved this from In Review to Done in CCCLSep 5, 2025
@rwgk
rwgk deleted the find_nvidia_headers_nvshmem branch September 5, 2025 19:21
@github-actions

Copy link
Copy Markdown
Doc Preview CI
Preview removed because the pull request was closed or merged.

@rwgk

rwgk commented Sep 5, 2025

Copy link
Copy Markdown
ContributorAuthor

For easy pre-release testing, I uploaded this to TestPyPI:

https://test.pypi.org/project/cuda-pathfinder/1.2.2.dev202509051239/

It's exactly the same as under this PR, except that I appended .dev202509051239 to the version number (version numbers have to be unique for PyPI, even TestPyPI, and cannot be overwritten).

$ git diff
diff --git a/cuda_pathfinder/cuda/pathfinder/_version.py b/cuda_pathfinder/cuda/pathfinder/_version.py
index 70aa6255c..6a6745b50 100644
--- a/cuda_pathfinder/cuda/pathfinder/_version.py
+++ b/cuda_pathfinder/cuda/pathfinder/_version.py
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
-__version__ = "1.2.2"
+__version__ = "1.2.2.dev202509051239"
$ rm -rf dist/
$ python -m build
$ twine check --strict dist/cuda_pathfinder-*-py3-none-any.whl
Checking dist/cuda_pathfinder-1.2.2.dev202509051239-py3-none-any.whl: PASSED
$ twine upload --verbose -r testpypi dist/*
View at:
https://test.pypi.org/project/cuda-pathfinder/1.2.2.dev202509051239/

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.pathfinderEverything related to the cuda.pathfinder modulefeatureNew feature or requestP0High priority - Must do!

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants

@rwgk@ZzEeKkAa@isVoid@kkraus14@leofang