oschulz / ValueShapes.jl

Duality of view between named variables and flat vectors in Julia
Other
12 stars 5 forks source link

Change ShapedAsNT to not be a 0-dim array #52

Closed oschulz closed 2 years ago

oschulz commented 2 years ago

Also support InverseFunctions.jl and ChangesOfVariables.jl.

codecov[bot] commented 2 years ago

Codecov Report

Merging #52 (807d683) into master (3372750) will decrease coverage by 4.51%. The diff coverage is 81.20%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
- Coverage   92.93%   88.41%   -4.52%     
==========================================
  Files          13       12       -1     
  Lines         708      829     +121     
==========================================
+ Hits          658      733      +75     
- Misses         50       96      +46     
Impacted Files Coverage Δ
src/value_accessor.jl 92.59% <33.33%> (-7.41%) :arrow_down:
src/array_shape.jl 83.58% <50.00%> (-11.50%) :arrow_down:
src/functions.jl 90.47% <50.00%> (-4.53%) :arrow_down:
src/value_shape.jl 81.48% <76.92%> (-10.36%) :arrow_down:
src/named_tuple_shape.jl 87.07% <81.42%> (-2.93%) :arrow_down:
src/named_tuple_dist.jl 89.93% <84.61%> (-1.74%) :arrow_down:
src/const_value_shape.jl 96.29% <90.00%> (-3.71%) :arrow_down:
src/const_value_dist.jl 97.14% <100.00%> (ø)
src/distributions.jl 82.75% <100.00%> (-8.16%) :arrow_down:
src/reshaped_dist.jl 100.00% <100.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3372750...807d683. Read the comment docs.

oschulz commented 2 years ago

CC @jknollm, @vindex10, @apmypb

This should fix our Zygote pullback problems, e.g. for MGVI.jl. Changing ShapedAsNT to not be a 0-dim array of NamedTuple allows for a cleaner approach in general, it also circumvents Zygote's special handling of arrays.

This also creates a clean separation between a NamedTuple and a ShapedAsNT (which now is mutable array-backed named-tuple-like type in it's own right). Users can now choose if they want distributions of NamedTuple or ShapedAsNT (with NamedTuple as the default).

This is breaking and will require some changes in BAT.jl to adapt to it - hopefully not too many, should be done soon.