[RFC] Enforce strides to be not NULL for non-zero ndims

[tqchen]

In the past, we allow DLTensor.strides to be NULL and use that to signal compactness. In practice, most frameworks will always export with strides for nonzero ndims. Enforcing strides to be not NULL would help with memory safety here. So this RFC proposes for us to enforce the strides to be not NULL.

[hpkfft]

If ndim == 0, then requiring both shape and strides to be NULL (instead of, for example, pointing to one past the end of some chunk of allocated memory) might be a good idea.

Regardless, as a general comment on the DLPack standardization process, I would very much like to see buy-in from the most important frameworks (e.g., NumPy) before anything is added to DLPack.
I'm not familiar with the NumPy code base, so I may be mistaken, but a cursory glance suggests to me that NumPy specifically sets strides to NULL if the tensor is compact and row-majored: dlpack.c

[tqchen]

Agree on the case of ndim==0 would be OK.

@rgommers @seberg maybe can help comment on numpy pov

[seberg]

No big opinion from me, Numpy always fills it in I think, but could also special case it for C contiguity. The current rule would mainly help frameworks that only have C contiguity, but without requiring NULL (so there is no need to confirm) its maybe just as well to force setting it.

[rgommers]

The proposal seems quite reasonable to me, and I agree with @seberg that it should be fine for NumPy. Even if there's a case where it'd be NULL currently, it shouldn't be hard to change that.

[seberg]

I'll just put it in to NumPy, since I doubt that anyone will complain.

Consumers can't rely on it anyway (it would be a minor speedup if they check for contiguous otherwise).

So the main annoyance will be for a producer that only has C-contiguous tensors. That isn't NumPy, though.
You are basically just shifting the annoyance from such a producer to the consumer, but maybe there are fewer such producers.

[tqchen]

seems like that we are converging towards adding it to the spec. I will send a PR about this

[tqchen]

#178

[leofang]

Has the transition plan to avoid segfault been discussed? All producer libraries would/could export a nullptr today for C-contiguous arrays, and the consumer libraries cannot just drop the if (strides == nullptr) check casually. There should be a plan similar to what's discussed for max_version on the Python side.

[seberg]

Right, just commented on the PR... I think the only clean thing is to bump to version 1.2 and say that after that it applies. At least it gives you an idea of when you can rely on it not being NULL.

(It would also be good to actually have a paper trail of changes in each version.)

[hpkfft]

Yes, I agree! I thought all along that we were discussing changes for a future version of the standard.
Currently, the standard allows NULL strides for C-contiguous arrays, and that cannot be changed retroactively.
(The standard does not require NULL for C-contiguous arrays, so producers can supply a valid strides array now, if they so choose.)

[seberg]

Ah right, I suppose it's effectively an unreleased change until it's tagged (a bit invisible, but OK, dunno if the readme could point to the current version?).
Should still bump the version in the header and mention the such a change in the header itself (for a while).

[tqchen]

Yes, we should bump a version, updated the note on the specific paper trail of change in v1.2