support conversion to AbstractArray

[daanhb]

Julia Base defines conversion to the abstract types AbstractArray{T} and AbstractArray{T,N} by invoking their (abstract) constructors. These fail with an error for static vectors (see #746).

This pull request defines the AbstractArray{T} and AbstractArray{T,N} constructors for static arrays in terms of similar_type. This enables the following:

julia> convert(AbstractVector{Float64}, SVector(1,2))
2-element SArray{Tuple{2},Float64,1,2} with indices SOneTo(2):
 1.0
 2.0

One use case is to generically change the element type of a vector, in the example above from Int to Float64.

I've added tests for SVector, SMatrix, MVector and MMatrix. Perhaps there are others?
Also, should the conversions perhaps be @inline?

[coveralls]

Coverage decreased (-0.1%) to 82.351% when pulling 057f68e on daanhb:master into c3686c4 on JuliaArrays:master.

[c42f]

This looks very neat, thanks.

It seems it should also work with the likes of Symmetric(SA[1 2; 2 3]) — can you add a small test for that (compare a couple of the types in StaticArrayLike).

[c42f]

Might be worth throwing in some @inferred here for some of these tests. For example:

Suggested change

	@test convert(AbstractArray{Float64}, sv) isa SVector{2,Float64}
	@test @inferred(convert(AbstractArray{Float64}, sv)) isa SVector{2,Float64}

[c42f]

I don't think you need the Tuple here as the existing constructor on line 5 should take care of this?

[andyferris]

Nice. I meant to do this years ago :)

[daanhb]

The plot thickens: I ended up testing all of the matrices in StaticMatrixLike (thanks for the suggestion) because some of them actually do not work. Chasing this, it works for Symmetric, Hermitian and Diagonal because they explicitly define the conversion to AbstractMatrix. See, e.g., here.

However, the other types (Transpose, Adjoint and the Triangular family) fall back to a default constructor in Base defined in array.jl, which invokes similar. Hence, an SArray becomes an MArray even with the current changes:

julia> tri = UpperTriangular(SA[1 2; 0 3])
2×2 UpperTriangular{Int64,SArray{Tuple{2,2},Int64,2,4}}:
 1  2
 ⋅  3

julia> convert(AbstractArray{Float64}, tri)
2×2 UpperTriangular{Float64,MArray{Tuple{2,2},Float64,2,4}}:
 1.0  2.0
  ⋅   3.0

At least it does not throw an error and the result is correct, just not optimal.

I've added tests for the types that do not completely work, but commented them out for now. This can not be fixed in StaticArrays and should probably be pursued in Base.

[daanhb]

Hmm, I thought about filing an issue with Julia, but it suddenly is not such a clear-cut case anymore. Construction of arrays has seen a great deal of discussion and people may have different expectations, but even regardless of that currently AbstractArray{T,N}(myarray) in Base typically returns something mutable. So, changing that would be breaking.

Worse, in fact it is breaking for StaticArrays too, because I did not realize before that this line actually works:

julia> using StaticArrays

julia> convert(AbstractVector{Float64}, SVector(1, 2))
2-element MArray{Tuple{2},Float64,1,2} with indices SOneTo(2):
 1.0
 2.0

The original example in #746 failed not because the constructor is not defined, but because the fallback in Base did not work for the non-isbits type BigFloat... In hindsight, that is exactly what the error message was telling me. D-oh! That also means the issue with my original error message was never the absence of the constructor, but the unrelated issue that assigment of BigFloat to mutable static arrays is not supported. (Which I guess is intentional.) I'll let myself out :-)

In my mind convert(AbstractArray{T,N}, myarray) would complement similar(myarray, T). similar deliberately throws away data and mutability, but the conversion would retain data and mutability. However, as things stand, by default and by design the conversion does retain data but also discards mutability. It can be, and actually mostly is, implemented in terms of similar and does not bring anything new to the table. That means I'm left looking for an idiomatic way to preserve both data and mutability :-)

A simple example unrelated to static arrays is the following:

julia> convert(AbstractVector{Float64}, 1:3)
3-element Array{Float64,1}:
 1.0
 2.0
 3.0

Which method would give me 1.0:1.0:3.0 instead?

[daanhb]

I guess one alternative for my original use case may be to just use broadcast:

julia> BigFloat.(SVector(1,2))
2-element SArray{Tuple{2},BigFloat,1,2} with indices SOneTo(2):
 1.0
 2.0

This typically doesn't preserve structure though:

julia> Float64.(1:3)
3-element Array{Float64,1}:
 1.0
 2.0
 3.0

[daanhb]

On second third thought, last comment for the day, things are still not entirely consistent as they are now without this pull request because conversion to a vector with the same element type returns itself even if it is immutable:

julia> convert(AbstractVector{Int}, SVector(1,2))
2-element SArray{Tuple{2},Int64,1,2} with indices SOneTo(2):
 1
 2

julia> convert(AbstractVector{Float64}, SVector(1,2))
2-element MArray{Tuple{2},Float64,1,2} with indices SOneTo(2):
 1.0
 2.0

The fallback conversion to a mutable container may just be because, well, one needs a mutable container in order to be able to copy data back into it.

[c42f]

That also means the issue with my original error message was never the absence of the constructor, but the unrelated issue that assigment of BigFloat to mutable static arrays is not supported. (Which I guess is intentional.)

This is arguably a bug but not really possible to fix efficiently and generically due to Julia's semantics of immutable and mutable objects. You may want a SizedArray or mutable FieldArray for this case instead. See, however, #749 where I've fixed this.

In my mind convert(AbstractArray{T,N}, myarray) would complement similar(myarray, T). similar deliberately throws away data and mutability, but the conversion would retain data and mutability.

This is how I expect it to behave: preserving "as much as possible" about the type of the source array, but changing its eltype. I reckon we just implement it this way. It may break some things, but only by removing mutability (so the breakage will be clearly visible). That's just as well, because depwarning this would be difficult!

[daanhb]

Just to be sure, I've checked what happens in Julia itself with conversions to AbstractArray. It is used very sparingly in Base, very liberally in the LinearAlgebra package, and just two times in SparseArrays. (This is based simply on grep, I could have missed many things.)

To test I've added a few lines (only three in total) to base/range.jl and stdlib/LinearAlgebra/src/triangular.jl, starting from Julia-1.4.0-rc2, to get the following behaviour:

julia> convert(AbstractVector{Float64}, 1:5)
1.0:1.0:5.0

julia> using LinearAlgebra

julia> tri = UpperTriangular(Diagonal(1:3))
3×3 UpperTriangular{Int64,Diagonal{Int64,UnitRange{Int64}}}:
 1  0  0
 ⋅  2  0
 ⋅  ⋅  3

julia> convert(AbstractMatrix{Float64}, tri)
3×3 UpperTriangular{Float64,Diagonal{Float64,StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}}:
 1.0  0.0  0.0
  ⋅   2.0  0.0
  ⋅    ⋅   3.0

julia> convert(AbstractMatrix{BigInt}, tri)
3×3 UpperTriangular{BigInt,Diagonal{BigInt,UnitRange{BigInt}}}:
 1  0  0
 ⋅  2  0
 ⋅  ⋅  3

In other words, the conversion preserves the immutable range type, and both UpperTriangular and Diagonal recursively invoke AbstractArray conversion on their data.

The results with static vectors would be similar, but I wanted to check Julia's test suite so I went for the immutable ranges.
The good news is that the entire test suite simply passes!

Two small things:

• I did have to restrict the element type in AbstractArray{T,1}(r::AbstractRange) to be <:Real because Ranges do not like complex numbers. This actually comes up in a test that does complex(1:5), and the conversion is triggered by this line in complex.jl.
• One conversion to AbstractArray in SparseArrays looks worrisome here. This code seems to assume that the result after conversion is mutable, even though the method itself is out-of-place. This would have led to an error, if a test had been included with an immutable type.

[daanhb]

In my mind convert(AbstractArray{T,N}, myarray) would complement similar(myarray, T). similar deliberately throws away data and mutability, but the conversion would retain data and mutability.

This is how I expect it to behave: preserving "as much as possible" about the type of the source array, but changing its eltype. I reckon we just implement it this way. It may break some things, but only by removing mutability (so the breakage will be clearly visible). That's just as well, because depwarning this would be difficult!

I agree with this, the breakage will be clearly visible - and hopefully limited. It would help if there was a generic function for the following two behaviours:

• give me the most efficient array that is like the one I have but with a different element type
• give me a mutable array that is like the one I have but with a different element type

The latter can be implemented purely in terms of similar, but the former can not and for optimal results it needs help from the specific concrete Array. (If it turns out to be helpful elsewhere, it could be part of the AbstractArray interface to optionally implement the AbstractArray{T,N} constructor this way..)

[c42f]

I guess we can we work around this by adding new constructors, eg, Transpose(m::StaticMatrix) = ...?

Either that or uncomment these and leave them as @test_broken for now?

[daanhb]

The construction of the matrices is fine, fortunately, with all these types, it is just the conversion. I think that, in order to fix it here, we would have to intercept functions like AbstractMatrix{T}(A::UpperTriangular{T,<:SArray}). That seems a bit intrusive, so I've added @test_broken for now like you suggest here. In the meantime I did file an issue with Julia (JuliaLang/julia#34995) to hopefully fix it upstream.

[c42f]

Defining AbstractMatrix{T}(A::UpperTriangular{T,<:SArray}) seems somewhat ok to me — we already do these kinds of things elsewhere in the package.

Having said that, it's definitely not ideal — thanks for submitting the upstream issue.