Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #626 +/- ##
==========================================
+ Coverage 93.91% 94.15% +0.24%
==========================================
Files 15 15
Lines 904 976 +72
==========================================
+ Hits 849 919 +70
- Misses 55 57 +2 ☔ View full report in Codecov by Sentry. |
oscardssmith
left a comment
There was a problem hiding this comment.
This is a pretty big change, but It looks good.
willtebbutt
left a comment
There was a problem hiding this comment.
This is cool. Glad to see it added. I've a few things to say about zero_tangent because I'm implemented something similar recently.
src/tangent_types/abstract_zero.jl
Outdated
| return Array{guess_zero_tangent_type(T),N} | ||
| end | ||
| guess_zero_tangent_type(::Any) = Any # if we had a general way to handle determining tangent type # https://github.com/JuliaDiff/ChainRulesCore.jl/issues/634 | ||
| # TODO: we might be able to do better than this. even without. No newline at end of file |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| # TODO: we might be able to do better than this. even without. | |
| # TODO: we might be able to do better than this. even without. |
|
It might make sense to test that this works for self-referential types c.f. something like this example from the docs. Maybe add a differentiable field of some kind? I'm not entirely sure how this will work -- I'm still trying to figure out how to avoid an infinite-loop in some of my work. |
|
I don't see any mention of self-referential types in the Swift docs on automatic tangent type synthesis, but I imagine they must've run into this so perhaps the implementation could have some clues? |
75c2ca3 to
7718bc7
Compare
|
Ok this has now been massively overhauled. However, in the process some things may have been broken. I expect MutableTangent will be missing some of the nicer overloads for linear operators etc., |
|
This property, in general, holds for all valid tangent types. |
|
This looks reasonable to me. |
|
The error in the projection tests is unrelated to this PR and is fixed in |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
f9ade6e to
95e63d0
Compare
Will close #105