ArrayInterface.jl icon indicating copy to clipboard operation
ArrayInterface.jl copied to clipboard

Bug in `strides` for reshaped views

Open ranocha opened this issue 4 years ago • 4 comments

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

  • ArrayInterface.strides(u_reshaped_view1) = (10 #= doesn't matter since there is only one element in this dimension =#, 10)
  • ArrayInterface.strides(u_reshaped_view1) = (10, 20)

Is that correct?

ranocha avatar Jun 02 '21 08:06 ranocha

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

Tokazama avatar Jun 02 '21 09:06 Tokazama

I will give it a shot.

ranocha avatar Jun 02 '21 10:06 ranocha

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.

Tokazama avatar Jun 06 '21 22:06 Tokazama

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

ranocha avatar Jun 07 '21 04:06 ranocha