ChainRulesCore.jl icon indicating copy to clipboard operation
ChainRulesCore.jl copied to clipboard

Collapse zeros for `Tuple` & `Ref` tangents

Open mcabbott opened this issue 3 years ago • 1 comments

This is one way not to make a Tangent with only zero types in it:

julia> ProjectTo(Ref(1))(Ref(1))  # ok
Tangent{Base.RefValue{Int64}}(x = 1.0,)

julia> ProjectTo(Ref(1))(Ref(NoTangent()))  # could collapse to NoTangent()
Tangent{Base.RefValue{Int64}}(x = NoTangent(),)

You could go further, and make Tangent{Foo}() and Tangent{Foo}(x = NoTangent()) all return NoTangent. But perhaps it's odd to make a a type constructor not return that type... this PR does not do that.

mcabbott avatar Jul 16 '22 01:07 mcabbott

Codecov Report

Merging #565 (971eba9) into main (f2e3ac5) will decrease coverage by 0.09%. The diff coverage is 88.88%.

@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
- Coverage   93.14%   93.04%   -0.10%     
==========================================
  Files          15       14       -1     
  Lines         875      877       +2     
==========================================
+ Hits          815      816       +1     
- Misses         60       61       +1     
Impacted Files Coverage Δ
src/projection.jl 96.98% <88.88%> (-0.33%) :arrow_down:
src/rule_definition_tools.jl 96.81% <0.00%> (-0.05%) :arrow_down:
src/ChainRulesCore.jl

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 f2e3ac5...971eba9. Read the comment docs.

codecov-commenter avatar Jul 16 '22 01:07 codecov-commenter

Xref discussion at #581, I guess that it would be slightly better for this to return a ZeroTangent than a NoTangent.

mcabbott avatar Sep 01 '22 20:09 mcabbott

The stronger way to abstract this would just be to make the Tangent constructor return a Zero. I don't quite see why an all-Zero Tangent is ever better.

mcabbott avatar Sep 05 '22 11:09 mcabbott