Support for ConstructorBase

[awadell1]

Is there any interest in adding support for ConstructorBase.constructorof to StaticArrays? I think it's as simple as adding the following, likely to convert.jl

ConstructionBase.constructorof(::Type{SA}) where {SA<:StaticArray} = SA

Best,

Alex

[mateuszbaran]

What would be the benefits of that? Static array construction is a tricky and convoluted part of StaticArrays.

[awadell1]

Hey Mateusz!

I'm using a struct to represent the parameters of a UODE, that gets solved by DifferentialEquations and then feed into ForwardDiff.jl to compute the gradiant of a loss function with respect to the model's parameters. Becuase ForwardDiff.jl requiries a vector and not a struct, I've been using Flatten.jl for the conversion.

struct p{T}
    a::T
    b::T
    c::NTuple{2, T}
end

using Flatten
param = p{Float64}(1, 2, (3, 4))
param_vec = [Flatten.flatten(param)...]

The problem arises when I try to use a StaticArray as a field, (Or a LSVector from LabeledArrays.jl). I want to do this because I need to normalize the state vector before feeding it into the NN.

Currently, this breaks down because Flatten.jl uses ConstructorBase.constructorof to determine how to reconstruct an object.

using StaticArray, Flatten
a = SVector(1, 2, 3)
Flatten.flatten(a)                      # Get (1, 2, 3) as expected
Flatten.reconstruct(a, (4, 5, 6)        # Expect SVector(4, 5, 6) -> get error

ERROR: DimensionMismatch("No precise constructor for SArray found. Length of input was 3.")
Stacktrace:
 [1] SArray(::Tuple{Tuple{Tuple{Tuple{Int64,Int64,Int64}}}}) at /home/awadell/.julia/packages/StaticArrays/l7lu2/src/convert.jl:1
 [2] StaticArray at /home/awadell/.julia/packages/StaticArrays/l7lu2/src/convert.jl:4 [inlined] (repeats 3 times)
 [3] macro expansion at /home/awadell/.julia/packages/Flatten/oXoLY/src/Flatten.jl:197 [inlined]
 [4] reconstruct(::SArray{Tuple{3},Int64,1,3}, ::Tuple{Int64,Int64,Int64}, ::typeof(flattenable), ::Type{Real}, ::Type{Flatten.EmptyIgnore}, ::Int64) at /home/awadell/.julia/packages/Flatten/oXoLY/src/Flatten.jl:193
 [5] reconstruct(::SArray{Tuple{3},Int64,1,3}, ::Tuple{Int64,Int64,Int64}, ::Function, ::Type{T} where T, ::Type{T} where T) at /home/awadell/.julia/packages/Flatten/oXoLY/src/Flatten.jl:186
 [6] reconstruct(::SArray{Tuple{3},Int64,1,3}, ::Tuple{Int64,Int64,Int64}, ::Function, ::Type{T} where T) at /home/awadell/.julia/packages/Flatten/oXoLY/src/Flatten.jl:184
 [7] reconstruct(::SArray{Tuple{3},Int64,1,3}, ::Tuple{Int64,Int64,Int64}, ::Function) at /home/awadell/.julia/packages/Flatten/oXoLY/src/Flatten.jl:183
 [8] reconstruct(::SArray{Tuple{3},Int64,1,3}, ::Tuple{Int64,Int64,Int64}) at /home/awadell/.julia/packages/Flatten/oXoLY/src/Flatten.jl:181
 [9] top-level scope at REPL[5]:1

The issue is that Flatten.jl replies on ConstructorBase.constructorof, and currently:

using ConstructorBase
ConstructionBase.constructorof(typeof(a))

Gives SArray, which does not reconstruct a and thus triggers the above errors. It looks like there has been some discussion (#759), but it doesn't currently seem to be a feature.

If this is a feature that y'all are interested in supporting, I'm happy to put together a pull request for it and work on the issue. Or if there's a better way to do this, I'm all ears (I'm still pretty new to Julia, so I might just be missing something).

Thanks for the awesome package,

Alex

[mateuszbaran]

OK, I see the problem here. I think you are very unlikely to convince people here to add a new dependency that would be required to add a new method (or, rather, many new methods) to constructorof. i would suggest putting these methods somewhere else.

Second issue is that ConstructionBase.constructorof(::Type{SA}) where {SA<:StaticArray} = SA is definitely not the correct solution (although it would work for many cases). You need to define it only for selected subtypes (my first guess is that for SArray{<:Tuple}, SArray{<:Tuple,T}, and similar variants for MArray and SizedArray). Some subtypes of SizedArray aren't even exactly constructible from stored data (for example sized views).

And thanks for bringing this issue here, constructing SizedArrays can be tricky. Let me know if you needed some help with that.

[rafaqz]

Also see: rafaqz/Flatten.jl#26

This is the correct solution:

using ConstructionBase, StaticArrays
struct SArrayConstructor{S,N,L} end
(::SArrayConstructor{S,N,L})(data::NTuple{L,T}) where {S,T,N,L} = SArray{S,T,N,L}(data)

ConstructionBase.constructorof(sa::Type{<:SArray{S,<:Any,N,L}}) where {S,N,L} = 
    SArrayConstructor{S,N,L}()

Repeated for MArray etc. SizedArray should be constuctable too.

I can put together a PR if this is a possibility. ConstructionBase is a small package if that counts for anything. It also means the object properties can be modified with Accessors.jl, Setfield.jl etc.

Additionally: putting methods somewhere else is also possible, maybe a small glue package that (with blessings) pirates ConstructionBase.constructorof for SArray, MArray and SizedArray. StraticArraysConstruction.jl ? It could live in JuliaObjects.

[jw3126]

OK, I see the problem here. I think you are very unlikely to convince people here to add a new dependency that would be required to add a new method (or, rather, many new methods) to constructorof. i would suggest putting these methods somewhere else.

@mateuszbaran I understand that there are objections to picking up dependencies. ConstructionBase is already a tiny package, but we could make the dependency even smaller to a package that only contains the lines:

function constructorof end
function setproperties end
function getproperties end

Would that solve the dependency issue for you? You might still have other objections, this question is only about the dependency.

[mateuszbaran]

Would that solve the dependency issue for you? You might still have other objections, this question is only about the dependency.

It isn't really my issue, I just described what I perceive as the general attitude towards making changes in StaticArrays.jl. Few people respond to issues here so I try to explain what I can.

Anyway, I think a StaticArraysConstruction.jl glue package is the right way to do here, at least for now. It would do a bit of type piracy but it shouldn't be an issue. If you need I can review the code.

[jw3126]

Thanks for the quick answer @mateuszbaran

Anyway, I think a StaticArraysConstruction.jl glue package is the right way to do here, at least for now. It would do a bit of type piracy but it shouldn't be an issue. If you need I can review the code.

One issue with that is that ConstructionBase is rarely used directly, but rather indirectly through Flatten.jl and friends. So having to import StaticArraysConstruction.jl for Flatten.jl to work correctly is unexpected for users.

[mateuszbaran]

One issue with that is that ConstructionBase is rarely used directly, but rather indirectly through Flatten.jl and friends. So having to import StaticArraysConstruction.jl for Flatten.jl to work correctly is unexpected for users.

Well, it would still be an improvement over the current situation, right?

[jw3126]

Well, it would still be an improvement over the current situation, right?

Yes, it would be an improvement. Mostly because is so easy to do better than everybody writing their own private pirate script.

[rafaqz]

There is also discussion in #901.

I'm open to a pirate glue package as last resort, it's definitely better than copy code snippets around as I have to recommend currently. But I do feel that a tiny dependency here would be the best solution long term to being able to modify these objects without a direct dependency on StaticArrays.jl, unless we get JuliaLang/Pkg.jl#1285.

A lot of the case for tools that manipulate immutable objects is as package dependencies. So the glue package can get a long way from the user, and intermediate packages risk throwing weird errors when a user hasn't loaded a glue package that they probably don't even know about. So the glue package wont scale very well.