way faster constructor

[YichengDWu]

Maybe I misunderstood, reduce looks like it's for restoring the type. If the type is determined at the beginning, then there is no need to call reduce.

julia> data_1 = rand(30,20);

julia> data_2 = rand(30,20);

julia> @benchmark ComponentArray(a = $data_1, b = $data_2) # Original
BenchmarkTools.Trial: 8075 samples with 1 evaluation.
 Range (min … max):  372.000 μs …   5.988 ms  ┊ GC (min … max):  0.00% … 90.97%
 Time  (median):     451.300 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   615.631 μs ± 819.858 μs  ┊ GC (mean ± σ):  25.07% ± 16.55%

  ██▄▃▁                                                     ▁ ▁ ▂
  █████▇▆▅▃▅▄▄▄▁▃▁▃▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▅████ █
  372 μs        Histogram: log(frequency) by time          5 ms <

 Memory estimate: 2.96 MiB, allocs estimate: 2442.

julia>  @benchmark ComponentArray(a = $data_1, b = $data_2) # This PR
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):   9.400 μs …  1.983 ms  ┊ GC (min … max): 0.00% … 98.03%
 Time  (median):     11.100 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   13.263 μs ± 49.256 μs  ┊ GC (mean ± σ):  9.68% ±  2.60%

     ▅▆▇▇█▆▆▅▅▂▂▁▁▂▁▂▃▂▂▂▃▁▂▁▂▁▁                              ▂
  ▂▃███████████████████████████████▇▇▇▅▆▇▇▆▆▆▇▆▆▆▆▃▄▅▅▅▅▄▆▅▄▅ █
  9.4 μs       Histogram: log(frequency) by time      22.7 μs <

 Memory estimate: 32.77 KiB, allocs estimate: 43.

[codecov-commenter]

Codecov Report

Merging #157 (18c36c0) into master (f3a1b6d) will increase coverage by 0.31%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
+ Coverage   76.62%   76.93%   +0.31%     
==========================================
  Files          20       20              
  Lines         586      594       +8     
==========================================
+ Hits          449      457       +8     
  Misses        137      137              

Impacted Files	Coverage Δ
src/componentarray.jl	82.40% <100.00%> (+0.43%)	⬆️
src/utils.jl	96.87% <100.00%> (+0.57%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

[jonniedie]

Thanks. This had been something I've been wanting to figure out for a while. I have a few minor requests for changes.

[YichengDWu]

The current method of creating a component array from a nested dict is a little strange, I think it's better to convert the dict into a named tuple first.

EDIT: just treat dicts like named tuples

[YichengDWu]

New benchmark

julia> @benchmark ComponentArray(a = $data_1, b = $data_2)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):   9.600 μs …  4.390 ms  ┊ GC (min … max):  0.00% … 99.16%
 Time  (median):     15.950 μs              ┊ GC (median):     0.00%
 Time  (mean ± σ):   20.259 μs ± 97.878 μs  ┊ GC (mean ± σ):  13.10% ±  2.95%

  ▂▇█▅▅▅▅▇▇▇▆▄▃▁▁           ▁▁▁▁  ▁▁▁▂▁                       ▂
  ███████████████████▇█▇▇▇████████████████▆▅▅▄▄▄▃▅▅▅▇▆▆▆▅▇▆▅▆ █
  9.6 μs       Histogram: log(frequency) by time      59.6 μs <

 Memory estimate: 42.25 KiB, allocs estimate: 43.

[avik-pal]

This change seems to have broken https://github.com/avik-pal/Lux.jl/runs/7702517164?check_suite_focus=true. @jonniedie could we add a downstream test dep on Lux similar to https://github.com/avik-pal/Lux.jl/blob/main/.github/workflows/Downstream.yml?

[YichengDWu]

You have some overloading there, I think it's time to upstream them instead.

[jonniedie]

@avik-pal yeah, that’s a good idea. And it would also be a good idea to upstream whatever Lux is overloading as well.

[YichengDWu]

I will do it.

This line is very domain specific @avik-pal, it could cause strange behaviour. Not so sure what is the motivation of it.

julia> ComponentArray(a=Int[], b=(;))
ComponentVector{Float32}(a = Float32[], b = Float32[])

julia> ComponentArray(a=Int[1], b=(;))
ComponentVector{Int64}(a = [1], b = Int64[])

[avik-pal]

I think at some point it would lead to a ComponentVector{Any} which is why I had that hacky thing. We should be able to remove that without any trouble

[YichengDWu]

Not sure if this PR would satisfy your requirement

[avik-pal]

Yeah it should do it LuxDL/Lux.jl#126