jonniedie / ComponentArrays.jl

Arrays with arbitrarily nested named components.
MIT License
287 stars 34 forks source link

Compatibility with StaticArrays? #183

Open JTaets opened 1 year ago

JTaets commented 1 year ago

I could really use something like a namedtuple that can do vector operations (immutable, indexable by name and no allocations when doing operations). My idea was to try out ComponentArrays with StaticArrays. But it didn't work

using ComponentArrays
using StaticArrays

comp = ComponentArray((a=5.,b=(c=7.,d=8.)))
scomp = ComponentArray(SVector(getdata(comp)...),getaxes(comp))
getdata(scomp)*2 #works
scomp*2 # Error

The error is:

ERROR: Dimension is not static. Please file a bug.
Stacktrace:
 [1] error(s::String)
   @ Base .\error.jl:35
 [2] copy
   @ C:\Users\x\.julia\packages\StaticArrays\jA1zK\src\broadcast.jl:61 [inlined]
 [3] materialize
   @ .\broadcast.jl:860 [inlined]
 [4] broadcast_preserving_zero_d
   @ .\broadcast.jl:849 [inlined]
 [5] *(A::ComponentVector{Float64, SVector{3, Float64}, Tuple{Axis{(a = 1, b = ViewAxis(2:3, Axis(c = 1, d = 2)))}}}, B::Int64)
   @ Base .\arraymath.jl:24
 [6] top-level scope
   @ c:\Users\x\Documents\julia3\test.jl:71 
jonniedie commented 1 year ago

Huh. This looks new. Static ComponentArrays have been possible, just not super well supported. I'm surprised it failed on something this basic, though. I'll take a look.

At some point I'd like to officially support StaticArrays by getting better test coverage and providing a constructor for a static ComponentArray so you do have to do the allocation on creation.

mikmoore commented 1 year ago

It seems that casting the axes of SVectors to SizedVectors results in SVector components. I haven't thought about how this interacts in multidimensional constructs, but SizedArray axes might be sufficient.

julia> using StaticArrays, ComponentArrays

julia> x = randn(SVector{3}); y = randn(4); z = randn(SVector{5});

julia> c = ComponentVector(vcat(x,y,z), Axis((;x=SizedVector{3}(1:3),y=4:7,z=SizedVector{5}(8:12))))
ComponentVector{Float64}(x = [-0.16252310061881806, -0.602345728457768, -0.6340903552211284], y = [-0.2714855111512396, 1.3403561914876867, -0.8518001483207325, 0.2835531733979676], z = [0.4911626462090142, 0.01824572353996267, -1.747235643671141, -1.5888066280127622, -0.5439915041247583])

julia> c[:x]
3-element SVector{3, Float64} with indices SOneTo(3):
 -0.16252310061881806
 -0.602345728457768
 -0.6340903552211284

julia> c[:y]
4-element Vector{Float64}:
 -0.2714855111512396
  1.3403561914876867
 -0.8518001483207325
  0.2835531733979676

julia> c[:z]
5-element SVector{5, Float64} with indices SOneTo(5):
  0.4911626462090142
  0.01824572353996267
 -1.747235643671141
 -1.5888066280127622
 -0.5439915041247583

julia> c.x # still get views from `getproperty` access, which seems fine
3-element view(::Vector{Float64}, [1, 2, 3]) with eltype Float64 with indices SOneTo(3):
 -0.16252310061881806
 -0.602345728457768
 -0.6340903552211284

If using only SVectors (i.e., leaving y out), the vcat results in SVector storage and the SizedVector axes result in SVector components (regardless of underlying storage). So it seems that the access functionality is sufficient and a constructor path is the main missing element.

jonniedie commented 1 year ago

Whoa! That's really cool! I've specifically wanted a way to unpack StaticArrays from a ComponentArray for a while, but every time I tried to implement the reconstruction by storing the input types, it turned out to be more difficult than I expected (the problem is there is no standard "construct from a type given arguments" that everyone follows and solutions like ReconstructionBase.jl don't work across the board). This seems like a really nice and minimal effort way to implement it, though. As long as we can assume all custom axis types will be isbits types, it should work to just store the axes on construction.

jonniedie commented 1 year ago

I just noticed I never fixed the original issue here too. I'll try to get to that this weekend.

jonniediegelman commented 1 year ago

The issue is that we're using the underlying array broadcast style and relying on the axes to recreate the ComponentArray rather than having our own broadcast style. So this line will obviously fail because the axes from the line above it is a special axis type and not a SOneTo. I really hope not to have to go back to using custom broadcast styles because the method ambiguity errors from trying to interoperate with other array packages were really bad.

jonniedie commented 12 months ago

Might be worth noting here: we export a new macro (#219) called @static_unpack that works like @unpack from UnPack.jl except it unpacks any array elements as StaticArrays. For my work, at least, I've found this to be more useful than having the ComponentArray be fully static.