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

document that DenseArrays do not need to define strides to follow the strided array API #54558

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LilithHafner
Copy link
Member

The true default definition, as per @less strides([]) is

strides(a::Union{DenseArray,StridedReshapedArray,StridedReinterpretArray}) = size_to_strides(1, size(a)...)

But I wrote an equivalent default that does not depend on internals.

@LilithHafner LilithHafner added the docs This change adds or pertains to documentation label May 22, 2024
@jishnub
Copy link
Contributor

jishnub commented May 23, 2024

I find the title a little confusing, since

julia> DenseArray <: StridedArray
true

Are StridedArrays not guaranteed to have strides defined for them?

Co-authored-by: Daniel Karrasch <[email protected]>
@LilithHafner
Copy link
Member Author

Are StridedArrays not guaranteed to have strides defined for them?

From reading the docs... no? The only thing I can see as guidance for declaring a new type <: DenseArray is

The elements of a dense array are stored contiguously in memory.

from the docstring of DenseArray. Maybe we should add a fallback definition for strides(a::StridedArray) or at least strides(a::DenseArray) based on eltype and size.

@LilithHafner LilithHafner changed the title document that DenseArrays do not need to define strides to be Strided Arrays. document that DenseArrays do not need to define strides to follow the strided array API May 23, 2024
@nsajko nsajko added the arrays [a, r, r, a, y, s] label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants