coverage: add cuda.core test_utils.py tests for DLPack/StridedMemoryView by rluo8 · Pull Request #2181 · NVIDIA/cuda-python

rluo8 · 2026-06-09T01:32:51Z

Description

Split out from #2168 per review. Adds tests targeting
cuda/core/experimental/_memoryview.pyx:

DLPack export round-trip over all NumPy-native dtypes and
scalar/zero-volume shapes
versioned/unversioned capsule + deleter paths
from_dlpack: ambiguous-stream and unsupported-device errors, TypeError -> unversioned-import fallback
CAI path: device-pointer + stream-ordering view, deprecated init path, has_dlpack=False proxy, device-accessible view, None-dtype repr
DLPack C exchange-API helpers

Coverage change:
_memoryview.pyx coverage 65.82% -> 85.73%.

github-actions · 2026-06-09T01:50:12Z

Doc Preview CI
🚀 View preview at https://nvidia.github.io/cuda-python/pr-preview/pr-2181/
https://nvidia.github.io/cuda-python/pr-preview/pr-2181/cuda-core/
https://nvidia.github.io/cuda-python/pr-preview/pr-2181/cuda-bindings/
https://nvidia.github.io/cuda-python/pr-preview/pr-2181/cuda-pathfinder/
Preview will be ready when the GitHub Pages deployment is complete.

seberg

I'd wait for @mdboom to have a brief look. As we discussed yesterday, I am not sure about all tests but I guess the improved coverage is really nice.
FWIW, in these changes I don't really see trivial gains for parametrization.

That said, I left a few comments and especially the TMD tests seem like a lot of effort to add coverage that should already exists (but you need the support to see that in the coverage).
I think it may make sense to add one test (which could use funky arguments) to check that an error is given when it isn't available. But I wouldn't add monkeypatch tests just to get coverage up here, I think.
(We should cover this with real tests not with dummy tests.)

The DLPack tests are pretty complex unittests with ctypes, I guess they are fine (this is unused, if there are bugs mirrored in implementation and tests we won't even notice, though -- which is a bit deceptive; it seemed fine but I'd have to look closer to be confident).
Either way, I feel this file is big enough and those tests complicated enough, that it may be good to split DLPack tests into a new one? (test_utils_dlpack.py or a new utils folder?)

seberg · 2026-06-10T07:02:01Z

+
+    The real ``_from_tiled`` requires a device-accessible, 16-byte-aligned view
+    on TMA-capable hardware (sm90+), so we replace the (module-level) class the
+    method imports with a recorder and assert the assembled call instead.


Not sure, but can't we just skip the test if the hardware isn't available? And if it is, I expect we are already testing this?

I feel this test belongs into test_tensormap as a single test that checks that tensormap instantiation fails gracefully when TMA isn't available.

Thanks for the thorough review, @seberg ! I'll update based on these comments.

Thanks @seberg . I have updated the test cases. Dropped both monkeypatch tests, and added one real test test_as_tensor_map_host_view_rejected_without_tma to test_tensor_map.py.

seberg · 2026-06-10T07:23:31Z

+    assert view.ptr == int(buffer.handle)
+    assert view.device_id == dev.device_id
+    assert view.shape == (8,)
+    dev.default_stream.sync()


I find this test a bit deceptive since it adds coverage without actually checking stream order correctness. (It does check for device correctness of course).
Now, testing stream-order correctness is annoying (you need either large buffers, or launch many kernels on the first stream and then e.g. a copy-to-host on after export and see that all of them finished)
So maybe it's OK, but the bot oversells the test by a lot.

Thanks for pointing this out. I've updated the docstring to be honest about what the test does and doesn't check.

seberg · 2026-06-10T07:26:08Z

+    """The deprecated ``StridedMemoryView(obj)`` constructor routes a CAI-only
+    object through the CAI branch (warn + ``view_as_cai``), not the DLPack one."""
+    obj = _make_cuda_array_interface_obj(shape=(4,), strides=None, typestr="<f4", data=(0, False))
+    with pytest.deprecated_call(match="deprecated"):


Suggested change

with pytest.deprecated_call(match="deprecated"):

with pytest.deprecated_call(match="CUDA-array-interface-supporting"):

if we bother to match a string, try to make it unique.

seberg · 2026-06-10T07:35:41Z

+#
+# Drive the C function pointers exposed by the capsule the way a native
+# consumer would, exercising the StridedMemoryView exchange-API implementation.
+# ---------------------------------------------------------------------------


We can add this, but I am starting to think this should be in it's own test_dlpack file, there is a lot of complexity here.

I have moved all the DLPack/exchange-API tests into a new file test_utils_dlpack.py.

seberg · 2026-06-10T07:46:04Z

+def _exchange_api_cause(exc):
+    """Underlying exception raised by the noexcept C fn (surfaced by ctypes as
+    SystemError, with the real error chained as __cause__ or __context__)."""
+    return exc.value.__cause__ or exc.value.__context__


Unpacking the SystemError is a bad workaround for not supplying an errcheck function to the ctypes definition.
We shouldn't test for SystemError (unless we set them ourselves, which we don't).

Thanks. I dropped the SystemError unpacking. The exchange-API function pointers are now declared with ctypes.PYFUNCTYPE instead of CFUNCTYPE, so ctypes surfaces the Python exception set by the Cython function directly. The tests now assert the intended TypeError/RuntimeError/NotImplementedError instead of inspecting SystemError.cause/context.

seberg · 2026-06-10T07:58:55Z

+    api = _get_exchange_api()
+    out = ctypes.c_void_p(123)
+    with pytest.raises(SystemError) as exc:
+        api.managed_tensor_allocator(None, ctypes.byref(out), None, None)


Suggested change

api.managed_tensor_allocator(None, ctypes.byref(out), None, None)

# Currently sets a Python error when `SetErr` isn't passed

api.managed_tensor_allocator(None, ctypes.byref(out), None, None)

This is OK, honestly, I am not sure if there is a point in even forcing this function to be non-NULL on the dlpack side if we just always make an error, but whatever.

I just prefer mentioning in the test that we know it's a weird test.

I have updated the test with suggested comments.

seberg · 2026-06-10T08:49:31Z

@@ -1094,3 +1094,425 @@ def test_strided_memory_view_dtype_roundtrip_all(dtype):
        pytest.skip(f"NumPy does not export {np.dtype(dtype)} via DLPack: {e}")
    view = StridedMemoryView.from_dlpack(src, stream_ptr=-1)
    assert view.dtype == np.dtype(dtype)  # .dtype triggers dtype_dlpack_to_numpy


Might as well delete this test and move the comment about bfloat16 to _NUMPY_NATIVE_DLPACK_DTYPES your new test seems basically identical but expands it a bit.

Done. Deleted the test, and moved the bfloat16 comment to _NUMPY_NATIVE_DLPACK_DTYPES.

seberg · 2026-06-10T12:22:20Z

+    np.complex64,
+    np.complex128,
+    np.bool_,
 )


Suggested change

)

)

if ml_dtypes is not None:

# Supported on NumPy 2.5 and ml_dtypes (probably) 0.5.5+

_NUMPY_NATIVE_DLPACK_DTYPES += (ml_dtypes.bfloat16,)

Although then the "native" is slightly off :). But we have the try/except that should make this skipped and NumPy 2.5 + next ml_dtypes version this should actually work.
(In fact, we can probably add more dtypes if we want to.)

Done. I added ml_dtypes.bfloat16 to _NUMPY_NATIVE_DLPACK_DTYPES when ml_dtypes is available. And unsupported NumPy/ml_dtypes combinations are still skipped by the per-test DLPack probe.

seberg · 2026-06-10T12:33:10Z

+    TypeError fallback in ``view_as_dlpack`` and an *unversioned* capsule import."""
+
+    def __init__(self, arr):
+        self._arr = arr


Suggested change

self._arr = arr

self._arr = StridedMemoryView.from_any_interface(arr)

I would suggest to use a StridedMemoryView here rather than NumPy. Should do the same thing, but means we don't care if NumPy changes the behavior (which eventually it may) and stops exporting version 0.x (unversioned).

Updated. Used a StridedMemoryView instead of NumPy, and passed stream_ptr=-1.

rwgk · 2026-06-17T20:03:05Z

Drive-by suggestion: @rluo8 could you please try adopting #2232 here, then ask your agent to add the tags? It'll be interesting to see if it works as intended.

coverage: add cuda.core test_utils.py tests for DLPack/StridedMemoryView

0b6efe3

github-actions Bot added the cuda.core Everything related to the cuda.core module label Jun 9, 2026

rluo8 self-assigned this Jun 9, 2026

rluo8 mentioned this pull request Jun 9, 2026

coverage: add cuda.core tests for host, launcher cluster, strided layout, program-cache, and DLPack utils #2168

Merged

leofang requested review from mdboom and seberg June 9, 2026 14:15

leofang added P1 Medium priority - Should do test Improvements or additions to tests labels Jun 9, 2026

leofang added this to the cuda.core v1.1.0 milestone Jun 9, 2026

Merge branch 'main' into coverage-test-utils

99d3f3f

seberg reviewed Jun 10, 2026

View reviewed changes

rluo8 added 6 commits June 12, 2026 02:11

Merge branch 'main' into coverage-test-utils

73b1f05

Address cuda.core coverage test review comments

f22ad8e

Fix pytest parametrize collection for CPU samples

c22e9de

Merge branch 'main' into coverage-test-utils

68191e4

coverage: refine cuda.core coverage tests comments

007cbdd

Merge branch 'main' into coverage-test-utils

87fd84a

	with pytest.deprecated_call(match="deprecated"):
	with pytest.deprecated_call(match="CUDA-array-interface-supporting"):

	api.managed_tensor_allocator(None, ctypes.byref(out), None, None)
	# Currently sets a Python error when `SetErr` isn't passed
	api.managed_tensor_allocator(None, ctypes.byref(out), None, None)

-)
+)
+if ml_dtypes is not None:
+    # Supported on NumPy 2.5 and ml_dtypes (probably) 0.5.5+
+    _NUMPY_NATIVE_DLPACK_DTYPES += (ml_dtypes.bfloat16,)

	self._arr = arr
	self._arr = StridedMemoryView.from_any_interface(arr)

Conversation

rluo8 commented Jun 9, 2026

Description

Uh oh!

github-actions Bot commented Jun 9, 2026

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

Uh oh!

seberg 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!

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!

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!

rwgk commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants