JuliaArrays / StaticArrayInterface.jl

Interface designs for enforcing static computations in array functions with Julia
MIT License
13 stars 4 forks source link

Bug in `strides` for reshaped views #2

Open ranocha opened 3 years ago

ranocha commented 3 years ago
julia> using ArrayInterface

julia> u_base = randn(10, 10); u_view = view(u_base, 3, :); u_reshaped_view1 = reshape(u_view, 1, :); u_reshaped_view2 = reshape(u_view, 2, :);

julia> ArrayInterface.strides(u_reshaped_view1)
(static(1), 1)

julia> ArrayInterface.strides(u_reshaped_view2)
(static(1), 2)

These seem to be wrong, I think. Because of ArrayInterface.strides(u_view) == (10,), the strides should be

Is that correct?

Tokazama commented 3 years ago

It does seem to be wrong. We probably need to have more robust support for ReshapedArrays overall in "stridelayout.jl"

ranocha commented 3 years ago

I will give it a shot.

Tokazama commented 3 years ago

I don't think we can actually guarantee that a ReshapedArray can combine with its parent strides unless we know that the the reshaped dimensions are contiguous and dense, because otherwise we would need to split strides. For example:

x = Array{Int}(undef, 5, 2, 5, 2); 
xv = view(x, :, 1, :, :);
xvr = reshape(xv, 10, 5)

If reshaping was done with static information we could figure this out at compile time, but with the current design of ReshapedArray we would have to compute all the parent strides and then see if one of the new dimensions is large enough to split parent strides.

ranocha commented 3 years ago

Yeah, that's like https://github.com/JuliaArrays/ArrayInterface.jl/issues/162#issuecomment-853109218