Skip to content

cuda.pathfinder._find_nvidia_header_directory(): add support for CTK libs #956

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 39 commits into from
Sep 17, 2025
Merged

cuda.pathfinder._find_nvidia_header_directory(): add support for CTK libs #956

merged 39 commits into from
Sep 17, 2025

Conversation

@rwgk
Copy link
Collaborator

@rwgk rwgk commented Sep 9, 2025 •
edited
Loading

Closes #832, #954

Make the cuda.pathfinder._find_nvidia_header_directory API public (by removing the leading underscore) and extend the function to also support CTK library headers.

See new find_nvidia_header_directory() docstring for details:

https://nvidia.github.io/cuda-python/cuda-pathfinder/1.2.3/generated/cuda.pathfinder.find_nvidia_header_directory.html

@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Sep 9, 2025

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
Copy link
Collaborator Author

rwgk commented Sep 9, 2025

/ok to test

@rwgk rwgk mentioned this pull request Sep 9, 2025
Closed
1 task
@leofang leofang added P0 High priority - Must do! feature New feature or request cuda.pathfinder Everything related to the cuda.pathfinder module labels Sep 9, 2025
@leofang leofang added this to the cuda-pathfinder parking lot milestone Sep 9, 2025
@github-actions

This comment has been minimized.

Sign in to view
@rwgk
Copy link
Collaborator Author

rwgk commented Sep 9, 2025

/ok to test

@leofang leofang requested review from leofang and shwina September 9, 2025 23:43
@leofang leofang linked an issue Sep 9, 2025 that may be closed by this pull request
[FEA]: Pathfinder: get include directory for where nvcc is hosted #954
Closed
1 task
leofang
leofang reviewed Sep 10, 2025
View reviewed changes
Copy link
Member

@leofang leofang left a comment

Choose a reason for hiding this comment

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

quick pass

@rwgk rwgk marked this pull request as ready for review September 16, 2025 15:00
@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Sep 16, 2025

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.

@rwgk
Copy link
Collaborator Author

rwgk commented Sep 16, 2025

@leofang I just marked this PR as Ready for review, but there is a small docs-only glitch that I couldn't resolve even after spending a good 45 minutes or so.

I added this part:

https://github.com/rwgk/cuda-python/blob/306684d4c99ad6e00df28172b99fc0f361b4838d/cuda_pathfinder/cuda/pathfinder/_headers/supported_nvidia_headers.py#L37-L40

But Sphinx doesn't pick it up. I looked around for a few minutes to see if there is an existing similar constant with documentation, but didn't get lucky. Do you know how to get Sphinx to pick up documentation for constants?

@leofang
Copy link
Member

leofang commented Sep 16, 2025

Q:

  1. Don't we usually put docstrings after the variables, like this?
module_level_variable = 12345
"""int: Module level variable documented inline."""
  1. I am a bit nervous about documenting private APIs. Is it a must?

@rwgk
Copy link
Collaborator Author

rwgk commented Sep 16, 2025

  1. Don't we usually put docstrings after the variables, like this?

Wrt commit 3faea76:

The triple-quoted docstring worked for Sphinx, but tripped up the check-docstring-first pre-commit check.

The #: markup works for Sphinx and keeps the pre-commit check happy.

I tried various ways to have the docstring in cuda/pathfinder/_headers/supported_nvidia_headers.py instead of __init__.py, but after spending 45 + 30 minutes unsuccessfully, I decided to give up and go with this solution. The generated HTML (local testing) looks good to me.

@rwgk
Copy link
Collaborator Author

rwgk commented Sep 16, 2025

/ok to test

leofang
leofang reviewed Sep 17, 2025
View reviewed changes
@rwgk
Copy link
Collaborator Author

rwgk commented Sep 17, 2025

/ok to test

@rwgk rwgk mentioned this pull request Sep 17, 2025
Merged
@rwgk
Copy link
Collaborator Author

rwgk commented Sep 17, 2025

Thanks for the reviews!

@rwgk rwgk merged commit d97a2cc into NVIDIA:main Sep 17, 2025
94 of 95 checks passed
@rwgk rwgk deleted the supported_nvidia_headers branch September 17, 2025 06:50
@github-actions
Copy link

github-actions bot commented Sep 17, 2025

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

This was referenced Sep 18, 2025
Merged
Merged
@rwgk rwgk mentioned this pull request Sep 25, 2025
Merged
@leofang leofang linked an issue Sep 26, 2025 that may be closed by this pull request
Add APIs for searching header paths #630
Closed
rwgk added a commit to rwgk/cuda-python that referenced this pull request Sep 29, 2025
@rwgk
Previously overlooked change that should have been included in PR NVI…
03a6628 
…DIA#956
@rwgk rwgk mentioned this pull request Sep 30, 2025
Merged
cpcloud pushed a commit that referenced this pull request Sep 30, 2025
@rwgk
Previously overlooked change that should have been included in PR #956 (
c4bb623 
#1050)
@leofang leofang mentioned this pull request Oct 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leofang leofang leofang approved these changes

@rparolin rparolin rparolin left review comments

@shwina shwina Awaiting requested review from shwina

Assignees

@rwgk rwgk

Labels

cuda.pathfinder Everything related to the cuda.pathfinder module feature New feature or request P0 High priority - Must do!

Projects

None yet

Milestone

cuda-pathfinder 1.3.0

3 participants

@rwgk @leofang @rparolin