Skip to content

Prepare for CTK 13.4 - #2423

Merged
mdboom merged 8 commits into
NVIDIA:mainfrom
mdboom:prepare-for-ctk-13.4
Jul 28, 2026
Merged

Prepare for CTK 13.4#2423
mdboom merged 8 commits into
NVIDIA:mainfrom
mdboom:prepare-for-ctk-13.4

Conversation

@mdboom

@mdboom mdboom commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

This updates the generated files with a number of bugfixes that were found while making CTK 13.4 work. We should merge this first so we can plainly see 13.4-only changes when we are really to work on that.

See MR 495 for the source of these changes.

This includes the addition of 4 new NVML APIs from CTK 13.3 that had never been implemented before.

@copy-pr-bot

copy-pr-bot Bot commented Jul 25, 2026

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.

@github-actions github-actions Bot added the cuda.bindings Everything related to the cuda.bindings module label Jul 25, 2026
@mdboom

mdboom commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom

mdboom commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@mdboom
mdboom marked this pull request as ready for review July 27, 2026 17:45
@mdboom
mdboom marked this pull request as draft July 27, 2026 18:10
@mdboom
mdboom marked this pull request as ready for review July 27, 2026 18:33
@mdboom mdboom self-assigned this Jul 27, 2026
@mdboom mdboom added enhancement Any code-related improvements P0 High priority - Must do! labels Jul 27, 2026
@mdboom mdboom added this to the cuda.bindings next milestone Jul 27, 2026
@mdboom

mdboom commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@github-actions

Copy link
Copy Markdown

@rwgk rwgk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Based on a codex gpt-5.6-sol analysis:

I took a closer look at the new raw-storage checks because I was unsure what behavior they were intended to cover.

For CUipcMemHandle_st(), neither CTK nor the driver performs the zero-initialization: the test does not call a CUDA API. The wrapper embeds a C struct in the Cython extension object, Cython/CPython zero-initializes that object storage before __cinit__, and __cinit__ merely points the wrapper at the embedded struct. There is no explicit clearing operation in cuda_bindings.

This makes the replacement for test_char_range somewhat questionable to me:

  • The old test exercised the exposed reserved setter and its char conversion behavior. Once that property is intentionally omitted, the replacement does not preserve the same coverage.
  • The new test primarily verifies Cython's object-allocation behavior by reaching through getPtr() with ctypes.
  • A default-constructed all-zero CUipcMemHandle_st is not itself a valid CUDA IPC handle, so the user-visible value of this invariant appears limited.

The graph tests are more valuable: their primary purpose is to ensure that returned edge-data wrappers outlive the temporary buffers used by the binding. However, I think their public-field checks and heap stirring already exercise that regression. Reading the hidden reserved bytes through ctypes is ancillary and hard-codes the eight-byte layout and the offset at which the reserved storage begins.

My recommendation would therefore be to delete test_char_range, retain the four graph lifetime tests, and remove only their raw reserved-byte assertions. If zero-initialized wrapper storage is intended to become an explicit cuda_bindings contract, I would prefer to make the initialization explicit in generated code and cover that contract with one focused test.

I do not consider this blocking for the PR and am leaving the final choice to you.

@mdboom

mdboom commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

My recommendation would therefore be to delete test_char_range, retain the four graph lifetime tests, and remove only their raw reserved-byte assertions. If zero-initialized wrapper storage is intended to become an explicit cuda_bindings contract, I would prefer to make the initialization explicit in generated code and cover that contract with one focused test.

I think this is a good idea. I had replaced the existing tests with like behavior on the assumption that otherwise would be seen as a reduction in test coverage. I absolutely agree that it's better to not enforce a contract we don't want to make. I'll make these changes and then merge.

@mdboom
mdboom enabled auto-merge (squash) July 28, 2026 17:30
@mdboom
mdboom merged commit 39c2813 into NVIDIA:main Jul 28, 2026
197 of 203 checks passed
github-actions Bot pushed a commit that referenced this pull request Jul 29, 2026
Removed preview folders for the following PRs:
- PR #2392
- PR #2423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements P0 High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants