Reviewing `ismutable` and other mutation / sparsity handling traits

La issue fa riferimento al docstring di ismutable, al koan del README e a esempi che coinvolgono predicati di ArrayInterface; inizia leggendo queste definizioni e confrontando i comportamenti degli array elencati. Il lavoro è completato quando si è arrivati a un significato approvato dai maintainer, a un comportamento di fallback, alla relazione con Base.ismutable e a indicazioni su quando questi predicati non dovrebbero essere usati, documentando tutto chiaramente.

Instead of griping on a slack thread & telling people to avoid this package, it was suggested that I make an issue.

I can see that there's some desire to answer the question "can my function work in-place on this array, or should I make a copy?" in a way that allows for Array, StaticArrays, CuArray, SparseArrays, etc. I think that's basically what this package would like to solve, and why it has 1716 dependents by now.

The entire documentation of ismutable appears to be this:

“”"
    ismutable(::Type{T}) -> Bool
Query whether instances of type T are mutable or not, see
https://github.com/JuliaDiffEq/RecursiveArrayTools.jl/issues/19.
“”"

There's also this koan in the readme:

Changed the default of ismutable(array::AbstractArray) = true. We previously defaulted to Base.@pure ismutable(array::AbstractArray) = typeof(array).mutable, but there are a lot of cases where this tends to not work out in a way one would expect.

What one would expect is, however, the whole question. There are quite a few possible answers:

1. true could mean that A[i,j] = x is guaranteed to work (for suitable x, and any i,j in the axes of A). Or perhaps just for some valid indices, not all of them.

2. It could mean that A[i,:] .= x is guaranteed to work (broadcasting not scalar, e.g. CuArrays), or that copyto!(A, B) is guaranteed to work for B of sufficiently similar type (e.g. copyto!(Diagonal(rand(3)), Diagonal(1:3))).

3. However, it could also mean that A[i,j] = x is a desirable way to work. Or that mul!(A, ...) is a good idea. Indexing might work for some sparse arrays, but be very slow. It will also work for self-aliased arrays, but might give weird answers. Maybe you'd like false to warn you off slow paths, or paths where return A isn't obviously the right answer, not just away from errors.

In any of these scenarios, I would guess that the default would be false, to mean "don't try working in-place, fall back to allocating". However:

4. It could be that false means you know there's a good out-of-place way to work. Returned only for SMatrix, etc. Default true will sometimes lead to errors.

5. It could also be that false means you can be sure nobody else will be mutating the array while you work. That's another possible purpose, like a recursive Base.ismutable. In which case the default, on unknown types, has to be true.

Each of these could be useful for someone asking the original "can my function work in-place on this array?" question. But to know whether ismutable answers the question for their particular function, they really need to know which it is.

So I think that the docstring has to very clearly tell us what to expect. What's guaranteed and what's not. What the fallback is and why. Why this isn't Base.ismutable (that one's easy, but still best to mention the name clash). For what purposes you should not use this, and instead use some other function, or just x isa Array.

My guess is that the intended meaning is somewhere between 1 and 2, but I'm not really sure. I presume any array package authors are equally unsure, and hence that how this function gets overloaded will be unreliable. Which is why I recommend against using this package, at present.

Here are some examples where true seems a bit surprising. Are these bugs or are they intended? And have other people spent 5 minutes trying to make up other adversarial examples?

julia> using ArrayInterface, JLArrays, OneHotArrays, ReadOnlyArrays, LinearAlgebra

julia> ArrayInterface.ismutable(UpperTriangular(rand(3,3)))  # can only write some places
true

julia> ArrayInterface.ismutable(Diagonal)  # abstract type, subtypes include Diagonal(1:3)
true

julia> ArrayInterface.ismutable(view(rand(3), [1,1,1]))  # self-aliased, writing will be weird
true

julia> ArrayInterface.ismutable(onehotbatch("abc", 'a':'z'))  # may soon allow setindex!, but a bit like self-aliasing
true

julia> ArrayInterface.ismutable(ReadOnlyArray(rand(3)))  # package designed to block writing
true

julia> ArrayInterface.ismutable(collect("abc")')  # cursed array, can neither read nor write
true

julia> ArrayInterface.can_setindex(jl(rand(3)))  # GPU array, scalar indexing will be bad (same for CuArray)
true