ArrayInterface.Size instead of ArrayInterface.size

[Tokazama]

This implements a formal type for extracting optionally static size info as I previously brought up here. If we do end up going this direction we can just leave a fall back to ArrayInterface.size(x) = Size(x).size so that we don't break any downstream code.

I think this is more beneficial than just creating a greater distinction between Base.size and ArrayInterface.size. It also allows passing Size as an unambiguous argument to methods like similar allocate a new array but currently dispatch on various combinations of integers and unit ranges for the dims argument. I'm hoping this could be used in combination with ArrayInterface.AbstractDevice to give us a more complete story for array constructors.

[codecov]

Codecov Report

Merging #241 (7c9e7bd) into master (75c9a32) will decrease coverage by 0.85%.
The diff coverage is 61.90%.

@@            Coverage Diff             @@
##           master     #241      +/-   ##
==========================================
- Coverage   89.45%   88.59%   -0.86%     
==========================================
  Files          11       11              
  Lines        1754     1780      +26     
==========================================
+ Hits         1569     1577       +8     
- Misses        185      203      +18     

Impacted Files	Coverage Δ
src/size.jl	71.91% <59.32%> (-15.97%)	⬇️
src/ArrayInterface.jl	90.47% <100.00%> (+0.01%)	⬆️
src/stridelayout.jl	87.12% <0.00%> (-0.24%)	⬇️
src/indexing.jl	85.92% <0.00%> (+0.06%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75c9a32...7c9e7bd. Read the comment docs.

[Tokazama]

@ChrisRackauckas, this is related to the issue you raised here JuliaLang/julia#25107

[ChrisRackauckas]

No it isn't, because Size holds a tuple.

[Tokazama]

Size would be equivalent to a dense constructor, so you could have something like:

construct(::CPUPointer, ::Type{T}, param::Size) where {T} = Array{T}(undef, size(param))
function construct(d::AbstractDevice, ::Type{T}, param::Banded) where {T}
    BandedMatrix(construct(d, T, Size(param)), bandwidths(param))
end

[ChrisRackauckas]

Why not go straight to making a AbstractConstructor system?

[Tokazama]

We could have Size <: AbstractConstructor and I'd actually prefer to have that, but due to concerns about compilation latency we've avoided adding a system that would naturally require too many additional types. I figured this approach would be a step in that direction without having to add on a whole new system all at once.

[chriselrod]

I'll have to look at this more closely, but would packages defining ArrayInterface.size methods still be handled correctly?

[ChrisRackauckas]

How are the downstream tests not failling?

[Tokazama]

I'll have to look at this more closely, but would packages defining ArrayInterface.size methods still be handled correctly?

That's the intention. If this just breaks a bunch of stuff then I don't think it would be worth implementing something like this until we have a more complete story for the whole construction interface.

[chriselrod]

How are the downstream tests not failling?

The tests pass if the compat of the downstream package excludes this ArrayInterface version.
At least in the case of LoopVectorization.jl, that is true (but sometimes Pkg thinks it is okay anyway?).

[Tokazama]

How are the downstream tests not failling?

Size(x).size should give the same exact result as the current ArrayInterface.size. Therefore, this PRs fallback to ArrayInterface.size(x) = Size(x).size shouldn't change anything.

[chriselrod]

That's the intention. If this just breaks a bunch of stuff then I don't think it would be worth implementing something like this until we have a more complete story for the whole construction interface.

The tricky bit is that we'd need both ArrayInterface.size and ArrayInterface.Size to work correctly, regardless of which one someone implemented. This is of course circular:

Size(x) = Size(size(x))
size(x) = Size(x).size

One solution would be to use static_hasmethod to break the circle.

[Tokazama]

Without an explicit method, Size uses the length of ArrayInterface.axes which in turn falls back to Base.axes, so it should only be circular if Base.axes(::ArrayType) depends on ArrayInterface.size(::ArrayType).

[chriselrod]

Without an explicit method, Size uses the length of ArrayInterface.axes which in turn falls back to Base.axes, so it should only be circular if Base.axes(::ArrayType) depends on ArrayInterface.size(::ArrayType).

Okay, that sounds fine.

[ChrisRackauckas]

@Tokazama could you please fix downstream. The way that recent changes has broken downstream makes it so that things cannot update and it is blocking the actual purpose of this package. If it doesn't get fixed soon I think we should just see adding some of the static stuff as a mistake and revert it / remove it all. It's supposed to be an interface for missing verbs, not something that is breaking constantly. I'd say by the end of next week is about how long to wait until it's very clear that that direction was just wrong.