`@grad`/`@grad_from_chainrules` silently reuse a tangent if a pullback returns too few tangents
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 83/100
Research direction
Start in src/macros.jl at the @grad and @grad_from_chainrules implementations linked in the issue, and check how each applies the pullback tangents. Look for the existing tests covering these macros. Done when both macros report an error if a pullback returns fewer tangents than there are arguments, rather than silently applying a tangent to another argument.
Written by the indexing model from the issue text.
Description
If a pullback returns fewer tangents than the function has arguments, ReverseDiff doesn't error. It silently reuses one tangent for every argument and returns a wrong gradient. This affects both @grad_from_chainrules and @grad.
using ReverseDiff, ChainRulesCore
f(x, y) = sum(x) + 10 * sum(y)
# malformed rrule: no tangent for `y`
ChainRulesCore.rrule(::typeof(f), x, y) = f(x, y), Δ -> (NoTangent(), fill(Δ, size(x)))
ReverseDiff.@grad_from_chainrules f(x::ReverseDiff.TrackedArray, y::ReverseDiff.TrackedArray)
ReverseDiff.gradient((x, y) -> f(x, y), ([1.0, 2.0], [3.0, 4.0]))
# ([1.0, 1.0], [1.0, 1.0])
g(x, y) = sum(x) + 10 * sum(y)
g(x::ReverseDiff.TrackedArray, y::ReverseDiff.TrackedArray) = ReverseDiff.track(g, x, y)
# malformed pullback: no tangent for `y`
ReverseDiff.@grad function g(x, y)
return g(ReverseDiff.value(x), ReverseDiff.value(y)), Δ -> (fill(Δ, size(x)),)
end
ReverseDiff.gradient((x, y) -> g(x, y), ([1.0, 2.0], [3.0, 4.0]))
# ([1.0, 1.0], [1.0, 1.0])
Expected: an error saying the pullback returned 1 tangent for 2 arguments. The gradient with respect to y is reported as [1.0, 1.0]. That's neither the true gradient ([10.0, 10.0]) nor anything the rule returned for y.
Cause: both macros apply tangents with _add_to_deriv!.(input, input_derivs) (@grad, @grad_from_chainrules). Broadcasting a 2-tuple against a 1-tuple expands the 1-tuple, so the x-tangent also goes to y. The @assert input_derivs isa Tuple there doesn't check the length.
ReverseDiff master (b796032, v1.18.4), Julia 1.13.1.
- Dominant language
- Julia
- Stars
- 396
- Forks
- 61
- Avg merge
- 23h 18m
- Merged PRs (30d)
- 16
Getting set up
This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from JuliaDiff/ReverseDiff.jl
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
JuliaDiff/ReverseDiff.jl#318 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
JuliaDiff/ReverseDiff.jl#314 ·
Maintainers usually reply within 1 day
-
Difficulty 4/5 3-5 days Newbie friendliness 56/100
JuliaDiff/ReverseDiff.jl#321 ·
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 72/100
JuliaDiff/ReverseDiff.jl#320 ·
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 68/100
JuliaDiff/ReverseDiff.jl#319 ·
Maintainers usually reply within 1 day
All issues in JuliaDiff/ReverseDiff.jl
Similar issues
-
documentation
Difficulty 2/5 Half a day Newbie friendliness 65/100
Maintainers usually reply within 6 days
-
broken links in docsOpen
Difficulty 1/5 Under an hour Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
JuliaPhysics/BeamletOptics.jl#127 ·
Maintainers usually reply within 1 day
-
Chains resumed from `initial_state` take `num_warmup + 1` warm-up stepsPossibly taken @thevolatilebit claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 80/100
TuringLang/AbstractMCMC.jl#220 ·