Skip to content

Conversation

@mdboom
Copy link
Contributor

@mdboom mdboom commented Jan 28, 2026

This is a small set of follow-on work that was missed in #1524 (which added a bunch of hand-written wrappers to cuda.bindings.nvml).

  1. One hand-written wrapper was missed: device_get_current_clock_freqs
  2. The handling of structs with fixed size arrays that are actually dynamically sized got some more testing on the code generator side, revealing a bug in the length checks. Those are fixed here.
  3. A few of the hand-written functions had incorrect return signatures. Cython doesn't really care about these, but for correctness these have been fixed.

@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Jan 28, 2026

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.

@mdboom mdboom requested a review from Copilot January 28, 2026 15:12
@mdboom
Copy link
Contributor Author

mdboom commented Jan 28, 2026

/ok to test

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR contains follow-on work to #1524 that was missed in the original implementation. The changes focus on: (1) adding the missed hand-written wrapper device_get_current_clock_freqs, (2) fixing length checks for structs with dynamically-sized fixed arrays, and (3) correcting return type signatures for several hand-written functions.

Changes:

  • Added hand-written wrapper for device_get_current_clock_freqs with proper string return type
  • Fixed array length validation in setters for BridgeChipHierarchy, ClkMonStatus, ConfComputeGpuCertificate, ConfComputeGpuAttestationReport, NvlinkSupportedBwModes_v1, and GridLicensableFeatures classes
  • Corrected return type signatures for multiple functions and moved version initialization inside nogil blocks for thread safety

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cuda_bindings/tests/nvml/test_device.py Updated test to expect string return type instead of struct for device_get_current_clock_freqs
cuda_bindings/cuda/bindings/_nvml.pyx Removed auto-generated DeviceCurrentClockFreqs_v1 class, added hand-written device_get_current_clock_freqs function, fixed array length checks in struct setters, corrected return type signatures, and moved version initialization inside nogil blocks
cuda_bindings/cuda/bindings/_nvml.pxd Added typedef for DeviceCurrentClockFreqs_v1 and removed old function declaration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

This comment has been minimized.

@mdboom mdboom enabled auto-merge (squash) January 28, 2026 15:55
@mdboom
Copy link
Contributor Author

mdboom commented Jan 28, 2026

/ok to test

@mdboom mdboom added bug Something isn't working feature New feature or request cuda.bindings Everything related to the cuda.bindings module labels Jan 28, 2026
@mdboom mdboom self-assigned this Jan 28, 2026
@mdboom mdboom added the to-be-backported Trigger the bot to raise a backport PR upon merge label Jan 28, 2026
@mdboom mdboom merged commit f4d3207 into NVIDIA:main Jan 28, 2026
94 checks passed
@github-actions
Copy link

Backport failed for 12.9.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 12.9.x
git worktree add -d .worktree/backport-1537-to-12.9.x origin/12.9.x
cd .worktree/backport-1537-to-12.9.x
git switch --create backport-1537-to-12.9.x
git cherry-pick -x f4d32072396a240a962eaf3ae108abd0536e9acf

@github-actions
Copy link

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

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

Labels

bug Something isn't working cuda.bindings Everything related to the cuda.bindings module feature New feature or request to-be-backported Trigger the bot to raise a backport PR upon merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants