Skip to content

cuda.core: consolidate driver version query functions#2040

Open
leofang wants to merge 3 commits intoNVIDIA:mainfrom
leofang:consolidate-driver-version
Open

cuda.core: consolidate driver version query functions#2040
leofang wants to merge 3 commits intoNVIDIA:mainfrom
leofang:consolidate-driver-version

Conversation

@leofang
Copy link
Copy Markdown
Member

@leofang leofang commented May 6, 2026 •
edited
Loading

Summary

Consolidate get_driver_version() and get_driver_version_full() into a single get_driver_version() that returns (umd_version, kmd_version) — a pair of version tuples (UMD is a 2-tuple (MAJOR, MINOR), KMD is a 3-tuple (MAJOR, MINOR, PATCH)).

The function now requires NVML and raises RuntimeError if it is not available.

  • Removes the kernel_mode parameter and the separate get_driver_version_full function
  • Updates tests, docs, and 1.0.0 release notes

@leofang @claude
cuda.core: consolidate get_driver_version and get_driver_version_full
8164b1b 
Merge the two separate driver version query functions into a single
get_driver_version() that returns (umd_version, kmd_version) — a pair
of version tuples. UMD is a 2-tuple (major, minor) and KMD is a
3-tuple (major, minor, patch). The function now requires NVML.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@copy-pr-bot
Copy link
Copy Markdown
Contributor

copy-pr-bot Bot commented May 6, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label May 6, 2026
@leofang leofang self-assigned this May 6, 2026
@leofang leofang requested a review from mdboom May 6, 2026 21:18
@leofang leofang added enhancement Any code-related improvements P0 High priority - Must do! breaking Breaking changes are introduced labels May 6, 2026
@leofang leofang added this to the cuda.core v1.0.0 milestone May 6, 2026
@leofang @claude
Remove unused driver import from _system.pyx
bfd9ab9 
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@leofang
Copy link
Copy Markdown
Member Author

leofang commented May 6, 2026

/ok to test bfd9ab9

@leofang leofang marked this pull request as ready for review May 6, 2026 21:33
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

Doc Preview CI

🚀 View preview at
https://nvidia.github.io/cuda-python/pr-preview/pr-2040/

https://nvidia.github.io/cuda-python/pr-preview/pr-2040/cuda-core/

https://nvidia.github.io/cuda-python/pr-preview/pr-2040/cuda-bindings/

https://nvidia.github.io/cuda-python/pr-preview/pr-2040/cuda-pathfinder/


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

@leofang @claude
Fix KMD version test to allow 2-tuple on WSL
d2d387e 
Windows driver version strings only have two components, so
nvmlSystemGetDriverVersion returns X.Y on WSL instead of X.Y.Z.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@leofang
Copy link
Copy Markdown
Member Author

leofang commented May 6, 2026

/ok to test d2d387e

rparolin
rparolin reviewed May 6, 2026
View reviewed changes
Comment thread cuda_core/tests/system/test_system_system.py
from .conftest import skip_if_nvml_unsupported


@skip_if_nvml_unsupported
Copy link
Copy Markdown
Collaborator

@rparolin rparolin May 6, 2026

Choose a reason for hiding this comment

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

Why does nvml have to be supported to check the driver version?

Copy link
Copy Markdown
Member Author

@leofang leofang May 6, 2026

Choose a reason for hiding this comment

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

Because this PR now uses NVML API calls exclusively.

Copy link
Copy Markdown
Collaborator

@rparolin rparolin May 6, 2026

Choose a reason for hiding this comment

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

Yeah, I'd argue there should be more traces to the reader to make that obvious. That coupled with the incorrect header at the top of the file make it difficult to parse.

rparolin
rparolin reviewed May 6, 2026
View reviewed changes
Comment thread cuda_core/tests/system/test_system_system.py
assert 0 <= ver_patch[0] <= 99
def test_driver_version_requires_nvml():
if system.CUDA_BINDINGS_NVML_IS_COMPATIBLE:
pytest.skip("NVML is available, cannot test the error path")
Copy link
Copy Markdown
Collaborator

@rparolin rparolin May 6, 2026

Choose a reason for hiding this comment

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

More comments explaining why NVML being available prevents testing would be a good trace for the reader.

Copy link
Copy Markdown
Member Author

@leofang leofang May 6, 2026

Choose a reason for hiding this comment

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

Because this test needs a negation for @skip_if_nvml_unsupported.

@rparolin
Copy link
Copy Markdown
Collaborator

rparolin commented May 6, 2026

Nit: the file-level comment at _system.pyx#L6-L8 still states that functions must "fall back to non-NVML-based methods for backward compatibility" when cuda.bindings.nvml isn't available. After this PR, get_driver_version() no longer follows that pattern (it raises RuntimeError instead), joining get_driver_branch() / get_nvml_version() which already required NVML. Worth tweaking the header to reflect that the file is now a mix — some functions fall back (e.g. get_num_devices), others require NVML.

rparolin
rparolin approved these changes May 6, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@rparolin rparolin left a comment

Choose a reason for hiding this comment

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

The changes are correct but some added context would make it obvious to reader why its correct. I've labeled them as nit to not block this PR.

@leofang
Copy link
Copy Markdown
Member Author

leofang commented May 6, 2026

Let me wait for @mdboom blessing before merging

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

Reviewers

@rparolin rparolin rparolin approved these changes

@mdboom mdboom Awaiting requested review from mdboom

Assignees

@leofang leofang

Labels

breaking Breaking changes are introduced cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P0 High priority - Must do!

Projects

None yet

Milestone

cuda.core v1.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@leofang @rparolin