Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix strides for 0-sized empty arrays #36

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

alexfikl
Copy link
Contributor

@alexfikl alexfikl commented Oct 1, 2021

For 0-sized arrays, this returned

>> c_contiguous_strides(8, (18, 0))
(0, 8)

which messes up cl.array.empty(queue, (18, 0), np.float64).strides. On the other hand, numpy gives

>>> np.empty((18, 0)).strides
(8, 8)

Not sure this is the right general fix, but it does the trick for this little example.

xref: inducer/arraycontext#91.

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! I think this is fine, but I'd like better comments! :) And code duplication is bad (sorry!).

array.py Show resolved Hide resolved
array.py Show resolved Hide resolved
@inducer
Copy link
Owner

inducer commented Oct 7, 2021

LGTM!

@inducer inducer merged commit 165b3ab into inducer:main Oct 7, 2021
@inducer
Copy link
Owner

inducer commented Oct 7, 2021

Can you pull this into the pyopencl change (inducer/pyopencl#514)? I'll pull it into pycuda and loopy.

@seberg
Copy link

seberg commented Oct 8, 2021

Super random, but NumPy indeed "ignores" size 1 dimensions when computing strides. However, NumPy will also when deciding whether strides are f or c-contiguous. So internally, numpy would consider the strides you had as valid f-contiguous strides (and will only sanitize this when exporting using the buffer protocol, which was necessary for cython at some point).

EDIT: To be clear, checking for contiguity, numpy completely ignores the stride value, and any stride value if the array is empty.

In that sense is_f_contiguous_strides is not the same as arr.flags.f_contiguous (it is not possible to do this by comparing strides only).

@inducer
Copy link
Owner

inducer commented Oct 8, 2021

Thanks for the heads-up! I've recorded this in #37.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants