OneTo indices in a view should have fast linear indexing

[Jutho]

I assume this was missed in the addition of the OneTo type?

B = view(A, indices(A,1), 2:2)

is contiguous and should have fast linear indexing. Does this need a test?

cc @timholy

[Jutho]

Clearly I was too optimistic in thinking that I could fix this via the Github web interface without testing it locally. I'll try to correct this when I am at home tonight.

[mbauman]

CI is just failing due to #18577. But more consideration will need to be given to offset arrays. If you want to do the minimal thing here and not worry about that, use Union{UnitRange, OneTo} instead of AbstractUnitRange since the latter might not start at one. Edit: Duh, UnitRanges already don't start at one. Pay no mind to me.

[timholy]

Good catch.

Interestingly, the example you give, however, can't have fast linear indexing. For example,

A = rand(3,5)
B = view(A, 1:2, 1:3)

clearly can't have fast linear indexing. Now, view(A, 1:3, 1:3) is contiguous and therefore could have fast linear indexing, but the type system (which is where we make this decision) doesn't know that.

However, if all trailing indices are scalars (2 rather than 2:2), then it's OK for the last "vector" index to be an AbstractUnitRange.

[Jutho]

Indeed, I was mixing linear indexing with the contiguous storage for BLAS matrix multiplication. But are the changes correct? Everywhere UnitRange is acceptable, OneTo should also be acceptable?

[timholy]

Yes, the code changes are perfect, and helpful indeed. Thanks!

From my perspective this can be merged once CI finishes.

[tkelman]

Does this need a test?

It wouldn't hurt?

[mbauman]

This is still worth doing. I've rebased it and added a test — although note that it doesn't fix the case @Jutho originally reported. We cannot assume that A[OneTo(4), 1:2] is linear as the first dimension might be larger than four elements. That's precisely why the special Base.Slice object exists — it flags the fact that we're spanning the entire dimension.

This means that Jutho's original case still must remain IndexCartesian. The case that this affects is OneTo indices that follow a complete slice:

view(A, indices(A,1), 2:2) # still IndexCartesian
view(A, :, indices(A,2)) # newly IndexLinear

[Jutho]

Thanks for that; I had forgotten about it. My initial example was indeed incorrect, as also pointed out by @timholy .