Construct natural tangents for Number and AbstractArray{<:Number} in forward mode #276
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This should also be called in |
staticfloat
reviewed
Mar 7, 2024
Comment on lines
+61
to
+66
| # TODO: should this use `_construct` ? | ||
| # TODO: is this right? | ||
| unwrap_tup(x::Tangent{<:Tuple}) = ChainRulesCore.backing(x) | ||
| unwrap_tup(x) = x | ||
| field_tangents = map(unwrap_tup, structural_tangent) | ||
| B(field_tangents...) |
There was a problem hiding this comment.
@Keno can you advise here?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ok.
Basically a lot of the code in ChainRules assumes that if your type is a
Numberthen its tangent is a number of that same type.A slightly smaller, but still non-zero portion of code assumed the same about
This is an alternative to
#272 and JuliaDiff/ChainRules.jl#787
Since if you always work with natural tangent types for
Numbers you don't run into problems like in ability to add them, since that same addition occurs in the primal problem.And that feels better, mostly.
I think it will also obsolete a lot of hackier things like our special handling for StaticArrays
e.g. what had to be changed in #275
we shouldn't need that rule at all. And probably some of the other rules in
extra_rules.jlcan go away.To support Diffractor over ForwardDiff we need both handling of natural tangents for
AbstractArray{<Number}(forForwardDiff.Partials) and handling of natural tangents forNumber(forForwardDiff.Dual, which usesForwarDiff.Partials)