-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DiffResults objects are not re-aliased properly #251
Labels
Comments
This was referenced Mar 15, 2024
See also JuliaDiff/ForwardDiff.jl#696 |
Here's an MWE demonstrating the bug: julia> using StaticArrays, ReverseDiff, DiffResults
julia> x = MVector{2}(3.0, 4.0);
julia> result_before = DiffResults.GradientResult(x)
ImmutableDiffResult(3.0, ([3.0, 4.0],))
julia> result_after = ReverseDiff.gradient!(result_before, sum, x)
ImmutableDiffResult(3.0, ([1.0, 1.0],)) # the first value should be 7 = sum(x)
julia> x = [3.0, 4.0];
julia> result_before = DiffResults.GradientResult(x)
MutableDiffResult(3.0, ([6.365987374e-314, 1.0e-323],))
julia> result_after = ReverseDiff.gradient!(result_before, sum, x)
MutableDiffResult(7.0, ([1.0, 1.0],)) # the first value is 7 = sum(x) |
This was referenced Jul 28, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The DiffResults documentation clearly states that whenever a
DiffResult
object is used, it must be realiased at the end of the function call:See for instance the docstrings in https://juliadiff.org/DiffResults.jl/stable/#Mutating-a-DiffResult, or the issue DiffResults#17
This is not done by ReverseDiff, for instance here:
ReverseDiff.jl/src/api/gradients.jl
Lines 21 to 27 in c982cde
Am I right in deducing that it can lead to incorrectness?
The text was updated successfully, but these errors were encountered: