Ship _v2 lowpp API for the driver module by mdboom · Pull Request #3095 · NVIDIA/cuda-python

mdboom · 2026-10-13T18:34:59Z

No description provided.

copy-pr-bot · 2026-10-13T18:35:03Z

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.

mdboom · 2026-10-13T19:09:22Z

/ok to test

mdboom · 2026-10-13T19:33:08Z

/ok to test

mdboom · 2026-10-13T19:38:29Z

/ok to test

mdboom · 2026-10-14T14:30:06Z

/ok to test cd9a1d8

mdboom · 2026-10-14T15:26:42Z

/ok to test 027a11e

github-actions · 2026-10-14T15:53:53Z

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

mdboom · 2026-10-14T16:09:16Z

/ok to test

mdboom · 2026-10-14T17:54:24Z

/ok to test

mdboom · 2026-10-15T17:56:01Z

/ok to test 650a8ab

mdboom · 2026-10-19T15:32:08Z

/ok to test 0c7b2a7

mdboom · 2026-10-19T15:35:37Z

/ok to test

mdboom · 2026-10-21T16:59:33Z

/ok to test dd186df

coderabbitai

Actionable comments posted: 9

🧹 Nitpick comments: 1

cuda_bindings_13/tests/test_cuda.py: 1
500-518: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

suggestion: Destroy graphExec before cleanup.

graphExec remains live until module context teardown. No later graph-memory assertion uses this context, so the measurable cross-test impact is not established.
+    driver.graph_exec_destroy(graphExec)
     driver.graph_destroy(graph)
     driver.stream_destroy_v2(stream)

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 08008359-6944-48d9-bf2e-acce3fd6e63d

📥 Commits

Reviewing files that changed from the base of the PR and between efd58e4 and dd186df.

📒 Files selected for processing: 32

.pre-commit-config.yaml
cuda_bindings_13/AGENTS.md
cuda_bindings_13/cuda/bindings/_v2/driver.pxd
cuda_bindings_13/cuda/bindings/_v2/driver.pyx
cuda_bindings_13/cuda/bindings/_v2/nvrtc.pxd
cuda_bindings_13/cuda/bindings/_v2/nvrtc.pyx
cuda_bindings_13/cuda/bindings/cudla.pyx
cuda_bindings_13/cuda/bindings/cufile.pyx
cuda_bindings_13/cuda/bindings/driver.pyx
cuda_bindings_13/cuda/bindings/nvfatbin.pyx
cuda_bindings_13/cuda/bindings/nvjitlink.pyx
cuda_bindings_13/cuda/bindings/nvml.pyx
cuda_bindings_13/cuda/bindings/nvvm.pyx
cuda_bindings_13/docs/source/api.rst
cuda_bindings_13/docs/source/module/driver_v2.rst
cuda_bindings_13/docs/source/module/nvrtc_v2.rst
cuda_bindings_13/docs/source/module/v2_migration.rst
cuda_bindings_13/docs/source/release/13.5.0-notes.rst
cuda_bindings_13/tests/conftest.py
cuda_bindings_13/tests/legacy_api/conftest.py
cuda_bindings_13/tests/legacy_api/nvml/test_legacy_nvml_cuda.py
cuda_bindings_13/tests/legacy_api/test_legacy_cuda.py
cuda_bindings_13/tests/legacy_api/test_legacy_cufile.py
cuda_bindings_13/tests/legacy_api/test_legacy_interoperability.py
cuda_bindings_13/tests/nvml/test_cuda.py
cuda_bindings_13/tests/test_cuda.py
cuda_bindings_13/tests/test_cufile.py
cuda_bindings_13/tests/test_interoperability.py
cuda_bindings_13/tests/test_kernelParams.py
cuda_bindings_13/tests/test_nvrtc.py
cuda_core/cuda/core/system/exceptions.py
cuda_core/docs/source/release/1.3.0-notes.rst

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

juenglin · 2026-10-22T21:15:18Z

Migration guide: are the suffixed legacy driver functions intentionally omitted?

The guide says the legacy module "exposes only the unsuffixed name" for versioned C symbols. But 8 suffixed legacy functions exist in driver.pyx (and on main) with no row in v2_migration.rst: cuCtxGetDevice_v2, cuCtxSynchronize_v2, cuGraphAddNode_v3, cuGraphNodeSetParams_v2, cuLaunchHostFunc_v2, cuMulticastBindAddr_v2, cuMulticastBindMem_v2, cuStreamGetCtx_v2.

This can mislead: cuCtxGetDevice → ctx_get_device_v2 is annotated "Parameters added: ctx", but a user already calling cuCtxGetDevice_v2(ctx) has no row for their actual function.

Deliberate, or should the tables add rows for the suffixed names?

juenglin · 2026-10-22T21:22:41Z

Naming nit on cu_memcpy (_v2/driver.pyx L30136) — posting at top level because GitHub won't take an inline comment on this generated file (diff too large).

cu_memcpy keeps the cu prefix, which no other function in this module does — the guide says the cu/nvrtc prefix is removed (e.g. cuMemcpyAsync → memcpy_async). Stripping it here would give memcpy. Was keeping cu_memcpy intentional, or should it follow the pattern?

juenglin

(Posting this first point in the review body: GitHub won't attach an inline comment to the 43k-line generated cuda_bindings_13/cuda/bindings/_v2/driver.pyx.)

_v2/driver.pyx TensorMap.opaque (~L10281) and the other view.array getters — use-after-free.

These getters do arr = cython.view.array(..., allocate_buffer=False); arr.data = &self._ptr[0].<member>; return _numpy.asarray(arr). The returned array's base is the cython array holding the raw pointer, with no strong reference back to self. Once the wrapper is garbage-collected the backing memory is freed while the view lives on (for TensorMap the struct is inline in the Python object).

Repro (no GPU needed):

from cuda.bindings._v2 import driver
import gc
views = []
for i in range(500):
    tm = driver.TensorMap()
    tm.opaque = [i] * 16
    views.append(tm.opaque)   # keep the view, drop the wrapper
    del tm
gc.collect()
flood = [driver.TensorMap() for _ in range(500)]   # reuse the freed 128B slots
for t in flood:
    t.opaque = [9999] * 16
print(sum(int(v[0]) == 9999 for v in views))        # -> 500: every view reads reused memory

Same pattern in .address_mode, .border_color, .max_threads_dim, .max_grid_size, and the two data_ getters. The recarray-backed structs already tie lifetime via owner=obj in from_ptr; these should too. A "view outlives wrapper" regression test would help — the existing test_array_member_assignment_and_aliased_dealloc only covers the raw-address+count members.

juenglin · 2026-10-22T21:26:36Z

+     -
+   * - ``nvrtcInstallBundledHeaders``
+     - ``install_bundled_headers``
+     - ``flags`` takes ``InstallHeadersFlag`` values. Returns the optional error message (``bytes`` or ``None``).


Doc vs. code mismatch. These rows say the bundled-header helpers return None on success, but the implementation returns b"" and the tests assert it: _v2/nvrtc.pyx:1054, :1081, :1115 and tests/test_nvrtc.py:43, :52, :54 (same for get_lowered_name at nvrtc.pyx:881). Please reconcile doc and code — a caller following the docs and writing if err is None would be wrong.

Good catch. Updated the docs to match the behavior in bdeddb0

juenglin · 2026-10-22T21:26:36Z

 InsufficientSizeError = nvml.InsufficientSizeError
 InsufficientPowerError = nvml.InsufficientPowerError
 DriverNotLoadedError = nvml.DriverNotLoadedError
+NvmlTimeoutError = nvml.TimeoutError


This rename doesn't actually stop the builtin shadowing. Here (and in the matching nvml.pyx) the old names are kept as aliases: cuda.bindings.nvml still defines TimeoutError = NvmlTimeoutError / MemoryError = NvmlMemoryError and has no __all__, and cuda.core.system re-exports both old names (this line plus MemoryError below, both added to __all__). So from ... import * still rebinds the builtin TimeoutError/MemoryError, which is the thing the release note says the rename fixes.

Broader question: the NVML rename and the regeneration of nvml/cufile/cudla/nvvm/nvjitlink/nvfatbin plus the legacy driver seem unrelated to the _v2 driver API work. Is bundling them here intentional, or would it be cleaner as a separate PR? They carry user-visible changes not called out in the description.

Unfortunately, we can't remove the old names without breaking API. Nor can we actually create deprecation warnings about their use (since they are used in a try/except and not actually called as functions). So the best we can do is add the new names as aliases now and declare them deprecated in the CHANGELOG, so we can remove the old names in the 14.x release.

An additional wrinkle is that cuda-core has to work with the latest /released/ version of cuda-bindings 13.4.x (not only the version here on main), so it has to continue to use the old names until 13.5.x is released and we can bump cuda-core's version floor on cuda-bindings.

Broader question: the NVML rename and the regeneration of nvml/cufile/cudla/nvvm/nvjitlink/nvfatbin plus the legacy driver seem unrelated to the _v2 driver API work. Is bundling them here intentional, or would it be cleaner as a separate PR? They carry user-visible changes not called out in the description.

These are due to bugs that were found during the course of the development of _v2.driver that also apply to these existing modules. (Bugs that only are found because the test coverage on driver is so much larger than any of the other modules). It would add complexity to the generator to generate in different ways for different modules, so we should be consistent.

However, it is a good callout that these changes (which are all backward-compatible) are not called out in the release notes. I have added them in e82804d.

juenglin · 2026-10-22T21:26:36Z

+  (``cuStreamCreate`` becomes ``stream_create``). Driver functions keep the version suffix of
+  the C symbol (``mem_alloc_v2``), whereas the legacy module drops it.
+- Parameters and struct members use ``snake_case``. Names that would clash with a Python keyword or
+  builtin, or with a name reserved by the generated classes, get a trailing underscore (``input_``, ``flags_``).


Rule doesn't match the generated names. This says names clashing with a Python builtin get a trailing underscore (input_, flags_), but several _v2.driver parameters keep builtin names: type (x11), object (x4), len (x2), bytes (x2), input (x2), format, map (input_ doesn't actually appear in driver.pyx). Since renaming later would be breaking, worth settling before the API firms up — adjust either the rule or the generator.

Yes, the note is wrong -- it's not about shadowing Python builtins, it's about avoiding shadowing Numpy methods that would shadow a field name. It turns out we don't /actually/ need to do this -- it's a bug -- but fixing it would breaking existing API, so it needs to be fixed by adding aliases to the old name, deprecating and then removing. For that reason, I think we should address it separately, and I will make an issue for the generator for that.

The note is fixed in 6d0109c

isVoid

My agent didn't find anything outside of the generated files region. I do have a small trivia below.

isVoid · 2026-10-22T21:23:25Z

+Enumerations
+------------
+
+Enumeration classes are ``int`` subclasses. Functions accept either the enumeration members or plain integers.
+Their members expose a description of the value automatically, so the individual members
+are no longer documented one by one.


I ran into similar agent design decision when doing rust bindings. Semantically C enums are closer to plain integers instead of python/rust enum. I find that interesting how different language bindings landed at the same design.

mdboom · 2026-10-23T14:19:06Z

Migration guide: are the suffixed legacy driver functions intentionally omitted?

No, that's just an omission in the migration guide. Fixed in 1e81c7e.

cu_memcpy keeps the cu prefix, which no other function in this module does

This was a good catch. My agent did this because it cimported the stdlib memcpy into the file and then thought this would shadow it. The solution is to rename the private/internal C stdlib memcpy, not the other way around. Fixed in c50ee15.

_v2/driver.pyx TensorMap.opaque (~L10281) and the other view.array getters — use-after-free.

This is describing an existing bug in all of the other cybind modules that has existed for a long time. It's a real issue, but may require some care and complexity to fix in an API-backward-compatible way. Since no one has actually reported the bug as an issue, I think this is lower priority and shouldn't hold up merging this PR. @juenglin already filed #3193 for this, so I think we just work on that separately.

juenglin

Looks good, thanks.

Removed preview folders for the following PRs: - PR #3095 - PR #3126 - PR #3173 - PR #3191 - PR #3197

Ship _v2 lowpp API for the driver module

39f66fa

github-actions Bot added cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module labels Oct 13, 2026

Merge remote-tracking branch 'upstream/main' into driver-cybind

d4d2fdb

Fix syntax

bd9188b

Fix tests

cd9a1d8

Fix test

027a11e

Fix thread behavior for legacy driver APIs

b2dd7a6

Fix ErrorError -> Error

f78fe24

Merge remote-tracking branch 'upstream/main' into driver-cybind

650a8ab

mdboom added 2 commits October 16, 2026 09:31

Merge in latest cybind changes

d501b94

Merge remote-tracking branch 'upstream/main' into driver-cybind

0c7b2a7

Merge remote-tracking branch 'upstream/main' into driver-cybind

eb77119

isVoid mentioned this pull request Oct 21, 2026

[BUG]: CUtensorMap owned storage does not honor required alignment #3181

Open

mdboom added 2 commits October 21, 2026 09:38

Merge remote-tracking branch 'upstream/main' into driver-cybind

d39eb4e

Fix readonly check

dd186df

mdboom marked this pull request as ready for review October 21, 2026 17:11

coderabbitai Bot reviewed Oct 21, 2026

View reviewed changes

mdboom added 3 commits October 21, 2026 14:12

Address CodeRabbit comment

c57ab3e

Fix NULL dereference found in review

edc2107

Merge remote-tracking branch 'upstream/main' into driver-cybind

7bc6799

mdboom requested review from isVoid and juenglin October 22, 2026 17:57

juenglin reviewed Oct 22, 2026

View reviewed changes

juenglin mentioned this pull request Oct 22, 2026

[BUG]: cuda.bindings array-member getters return NumPy views that can outlive their owning wrapper (use-after-free) #3193

Open

isVoid approved these changes Oct 22, 2026

View reviewed changes

mdboom added 5 commits October 23, 2026 09:41

Fix issues in migration doc

1e81c7e

Move cu_memcpy -> memcpy

c50ee15

Make docs match behavior

bdeddb0

Update release notes

e82804d

Fix comment about name shadowing

6d0109c

mdboom requested a review from juenglin October 23, 2026 14:19

Merge remote-tracking branch 'upstream/main' into driver-cybind

ded783d

juenglin approved these changes Oct 23, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/main' into driver-cybind

32adb93

mdboom enabled auto-merge (squash) October 23, 2026 17:54

mdboom added 3 commits October 23, 2026 14:13

Fix test after merge

ceefdfc

Fix link

a8fb3a8

Attempt to fix test

39fdb58

mdboom merged commit 2da9c65 into NVIDIA:main Oct 24, 2026
118 checks passed

github-actions Bot pushed a commit that referenced this pull request Oct 24, 2026

Clean up PR preview folders for 5 closed/merged PRs

243374d

Removed preview folders for the following PRs: - PR #3095 - PR #3126 - PR #3173 - PR #3191 - PR #3197

jpascucci-nv mentioned this pull request Oct 26, 2026

[BUG]: Free-threaded CI: coredump bool regression test races on application-wide settings #3225

Closed

1 task done

coderabbitai Bot mentioned this pull request Oct 26, 2026

Fix coredump tests on free-threaded builds #3226

Merged

mdboom mentioned this pull request Oct 27, 2026

gh-pages deploy failing: artifact crossed Pages publish cap on Oct 26 #3231

Open

Conversation

mdboom commented Oct 13, 2026

Uh oh!

copy-pr-bot Bot commented Oct 13, 2026

Uh oh!

mdboom commented Oct 13, 2026

Uh oh!

mdboom commented Oct 13, 2026

Uh oh!

mdboom commented Oct 13, 2026

Uh oh!

mdboom commented Oct 14, 2026

Uh oh!

mdboom commented Oct 14, 2026

Uh oh!

github-actions Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdboom commented Oct 14, 2026

Uh oh!

mdboom commented Oct 14, 2026

Uh oh!

mdboom commented Oct 15, 2026

Uh oh!

mdboom commented Oct 19, 2026

Uh oh!

mdboom commented Oct 19, 2026

Uh oh!

mdboom commented Oct 21, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

juenglin commented Oct 22, 2026

Uh oh!

juenglin commented Oct 22, 2026

Uh oh!

juenglin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isVoid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdboom commented Oct 23, 2026

Uh oh!

juenglin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

github-actions Bot commented Oct 14, 2026 •

edited

Loading