Prefer numeric zero over ZeroTangent for numeric arrays #1524
Conversation
This file contains 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
|
I believe the failing test is unrelated. |
|
Do you mind force pushing (good change to rebase and squash too)? Just so we can double-check the GPU tests are unaffected since they failed to run properly last time. |
Add tests Fix tests
BioTurboNick
force-pushed
the
number-zero-tangent
branch
from
74b034d
to
d4bf828
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If Zygote produces an
Array{Union{Nothing, <: Number}}, when passed throughwrap_chainrules_input, produces anArray{Any}because thenothingobjects are mapped toZeroTangentobjects. However, ChainRules uses thezeroof the type whenever possible so that the array maintains a concrete element type. This PR provides such a method.Related to JuliaDiff/ChainRules.jl#807
Note: It's possible that this case was only possible due to an upstream issue. Currently, I suspect that when Zygote provides an array with Tangents with NamedTuple components and ZeroTangents, the components of the NamedTuple are extracted and the ZeroTangent is mapped to
nothinginstead of performing thezero(T)optimization. But I wasn't able to track down the responsible code yet.Also added tests to cover the related methods.
PR Checklist