feat: add from_* style constructor classmethods to StridedMemoryView and make constructor amenable to future from_*-style constructors #1224

cpcloud · 2025-11-07T21:22:01Z

This PR adds three methods to conform better to APIs familiar to Python users and to ease future work on StridedMemoryView
adding support for constructing them from other objects besides CAI and dlpack objects.

Three methods are added:

from_dlpack(obj, stream_ptr) same functionality as before
from_cuda_array_interface(obj, stream_ptr) same functionality as before
from_any_interface(obj, stream_ptr) same behavior as the previous constructor

The constructor changes to a simple field initialization, in an effort to make
it easier to add a from_buffer (naming TBD) method that allows construction
from an owned raw buffer (without shape and stride information), which may
require passing in additional information such as shape and stride to the
constructor.

copy-pr-bot · 2025-11-07T21:22:05Z

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.

cpcloud · 2025-11-07T21:22:05Z

/ok to test

cpcloud · 2025-11-07T21:38:44Z

/ok to test

cpcloud · 2025-11-07T21:39:53Z

/ok to test

cpcloud · 2025-11-10T15:58:21Z

/ok to test

cpcloud · 2025-11-10T15:59:10Z

/ok to test

mdboom

LGTM, other than the strides property issue.

cuda_core/cuda/core/experimental/_memoryview.pyx

cpcloud · 2025-11-10T17:01:36Z

/ok to test

cpcloud · 2025-11-10T17:08:02Z

/ok to test

cpcloud · 2025-11-10T17:12:22Z

/ok to test

cpcloud · 2025-11-10T17:17:45Z

cuda_core/cuda/core/experimental/_memoryview.pyx


    @property
-    def strides(self) -> Optional[tuple[int]]:
+    def strides(self) -> tuple[int, ...]:


Oops, let me fix the annotation.

…and make constructor amenable to future from_*-style constructors

cpcloud · 2025-11-10T17:18:59Z

/ok to test

mdboom

LGTM

github-actions · 2025-11-10T19:00:37Z

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

leofang · 2025-11-10T19:11:27Z

cuda_core/cuda/core/experimental/_memoryview.pyx

-    def __init__(self, obj=None, stream_ptr=None):
-        if obj is not None:
-            # populate self's attributes
-            if check_has_dlpack(obj):
-                view_as_dlpack(obj, stream_ptr, self)
-            else:
-                view_as_cai(obj, stream_ptr, self)


@cpcloud This is a breaking change. Can we please restore the old behavior and add a deprecation warning? We need a release note, too.

If we want to push for alternative constructors, we should eventually raise in __init__ following what we do in other objects.

Can you point me to downstream breakages so I can understand the impact? Across all of GitHub the only use of StridedMemoryView is in a comment:

https://github.com/search?q=StridedMemoryView+NOT+is%3Afork+NOT+repo%3ANVIDIA%2Fcuda-python+NOT+owner%3APopcornFX&type=code

I'd strongly prefer not to restore the behavior without evidence that this change causes problems.

Happy to add the release note.

We have real users that cannot be found this way. Examples include issues filed by people outside of the team:

Lowering StridedMemoryView attribues to typed efficient C/C++/Cython accessible values #180

[BUG]: Segmentation fault with StridedMemoryView.shape #1186

[BUG]: StridedMemoryView leaks the producer memory #1043

BUG: StridedMemoryView fails to wrap NumPy array #473

StridedMemoryView fails with Jax arrays #285 (comment)

We have way too many invisible users at this point (check out the download counts from e.g. pypistats). We have also been slammed by our PM for breaking too often and too much (despite I disagreed in that case; in any case I was the one arguing and taking time in sorting things out, so plz back me up).

We need to make sure major breaking changes like this have a deprecation period, even before GA. In particular, in this case it is absolutely fine that we add extra constructors without breaking anything. I am not sure why we must leap so aggressively.

+1 to @leofang guidance. Breaking existing public facing API should go through our teams standard deprecation flow where we notify users at least a release before to when we remove the code.

Apologies for approving while misunderstanding the backward-compat guarantees of the experimental namespace.

I think this is all a reasonable policy to have, but challenging to follow as long as we are releasing from main. If we had a maintenance branch, we could put the "backward-compatible-with-deprecation-warnings" version of this on the maintenance branch, and the aspirational "where-we-want-to-be" on main. As it stands, we need to do the first now, put a reminder in a calendar to do the second and hope that reviews get it merged in the expected window.

Sorry, I've meant to follow up here and offline with Phillip, but I got distracted (as always) by other things including the pixi stuff that Phillip started playing around with 😛

I did not mean that this PR is bad. I apologize if my earlier comments gave this impression. The alternative constructors are totally fine. They give users finer control if they already have prior knowledge on what protocols a foreign object supports. I meant that we just need to preserve the default constructor's behavior for a little longer, at least for another minor release. We could just revert the change on __init__() and leave everything else as-is, though now that we have an urgent (TBD) patch release in preparation, it is good that it's reverted for now.

We're dealing with an interesting situation. On the one hand we're still beta/experimental software, and we do allow breaking changes in each minor release. On the other hand we are perhaps too successful in telling our CUDA Python story that we have unexpectedly high number of users, and this does increasingly keep me up at night. If we break too often, downstream projects end up pinning too tightly (example), resulting in NVIDIA software unable to be installed in the same environment. This hurts our storytelling.

I suggest that we clearly highlight all PRs that contain breaking changes with the "breaking" label and a rel-note entry under the Breaking Change section. With such marks we can make informed decisions and evaluate all breaking changes on a case-by-case basis. There is also no need to seek short turnaround in merging breaking PRs. We still want to design cuda.core properly in a future-looking fashion to avoid as much breaks as possible, it's been one of the core principles since Day 1, and for that I am fine taking a bit more time thinking through the design.

I didn't interpret your comments as this PR being bad, just haven't had a chance to re-PR the changes while preserving the current constructor behavior. I will PR that today.

…oryView and make constructor amenable to future from_*-style constructors (NVIDIA#1224) This reverts commit f223c17.

…oryView and make constructor amenable to future from_*-style constructors (#1224) (#1232) This reverts commit f223c17.

…ridedMemoryView and make constructor amenable to future from_*-style constructors (NVIDIA#1224) (NVIDIA#1232) This reverts commit a87f937.

This comment has been minimized.

Sign in to view

cpcloud force-pushed the smv-info-constructor branch from 786140a to f7eef8d Compare November 7, 2025 21:38

cpcloud force-pushed the smv-info-constructor branch from f7eef8d to 2cd276a Compare November 7, 2025 21:39

cpcloud force-pushed the smv-info-constructor branch from 2cd276a to 35bcd87 Compare November 10, 2025 15:56

cpcloud force-pushed the smv-info-constructor branch from 35bcd87 to 6f8ff96 Compare November 10, 2025 15:59

mdboom requested changes Nov 10, 2025

View reviewed changes

cuda_core/cuda/core/experimental/_memoryview.pyx Outdated Show resolved Hide resolved

cuda_core/cuda/core/experimental/_memoryview.pyx Outdated Show resolved Hide resolved

cuda_core/cuda/core/experimental/_memoryview.pyx Outdated Show resolved Hide resolved

cpcloud force-pushed the smv-info-constructor branch from 6f8ff96 to 28727f8 Compare November 10, 2025 17:01

cpcloud force-pushed the smv-info-constructor branch from 28727f8 to 7620fee Compare November 10, 2025 17:06

cpcloud force-pushed the smv-info-constructor branch from 85a81a9 to 0c267bc Compare November 10, 2025 17:12

cpcloud requested a review from mdboom November 10, 2025 17:16

cpcloud commented Nov 10, 2025

View reviewed changes

feat: add from_* style constructor classmethods to StridedMemoryView …

08676b1

…and make constructor amenable to future from_*-style constructors

cpcloud force-pushed the smv-info-constructor branch from 0c267bc to 08676b1 Compare November 10, 2025 17:18

mdboom approved these changes Nov 10, 2025

View reviewed changes

cpcloud merged commit f223c17 into NVIDIA:main Nov 10, 2025
102 of 103 checks passed

cpcloud deleted the smv-info-constructor branch November 10, 2025 18:51

leofang reviewed Nov 10, 2025

View reviewed changes

leofang assigned cpcloud Nov 10, 2025

leofang added the P0 High priority - Must do! label Nov 10, 2025

leofang added feature New feature or request cuda.core Everything related to the cuda.core module breaking Breaking changes are introduced labels Nov 10, 2025

leofang added this to the cuda.core beta 9 milestone Nov 10, 2025

cpcloud mentioned this pull request Nov 11, 2025

revert: feat: add from_* style constructor classmethods to StridedMemoryView and make constructor amenable to future from_*-style constructors (#1224) #1232

Merged

rwgk pushed a commit that referenced this pull request Nov 11, 2025

revert: feat: add from_* style constructor classmethods to StridedMem…

a87f937

…oryView and make constructor amenable to future from_*-style constructors (#1224) (#1232) This reverts commit f223c17.

leofang removed P0 High priority - Must do! feature New feature or request breaking Breaking changes are introduced labels Nov 15, 2025

leofang removed this from the cuda.core beta 10 milestone Nov 15, 2025

cpcloud mentioned this pull request Nov 17, 2025

feat: add from_* style constructor classmethods to StridedMemoryView and deprecate __init__ constructor #1250

Open

feat: add from_* style constructor classmethods to StridedMemoryView and make constructor amenable to future from_*-style constructors #1224

feat: add from_* style constructor classmethods to StridedMemoryView and make constructor amenable to future from_*-style constructors #1224

Uh oh!

Conversation

cpcloud commented Nov 7, 2025

Uh oh!

copy-pr-bot bot commented Nov 7, 2025

Uh oh!

cpcloud commented Nov 7, 2025

Uh oh!

This comment has been minimized.

cpcloud commented Nov 7, 2025

Uh oh!

cpcloud commented Nov 7, 2025

Uh oh!

cpcloud commented Nov 10, 2025

Uh oh!

cpcloud commented Nov 10, 2025

Uh oh!

mdboom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cpcloud commented Nov 10, 2025

Uh oh!

cpcloud commented Nov 10, 2025

Uh oh!

cpcloud commented Nov 10, 2025

Uh oh!

cpcloud Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

cpcloud commented Nov 10, 2025

Uh oh!

mdboom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Nov 10, 2025

Uh oh!

leofang Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

cpcloud Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leofang Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

rparolin Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

mdboom Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

leofang Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

cpcloud Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cpcloud Nov 10, 2025 •

edited

Loading