Skip to content
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

Derivative lost in 3-arg mul #536

Closed
antoine-levitt opened this issue Jul 19, 2021 · 4 comments · Fixed by #481
Closed

Derivative lost in 3-arg mul #536

antoine-levitt opened this issue Jul 19, 2021 · 4 comments · Fixed by #481

Comments

@antoine-levitt
Copy link
Contributor

antoine-levitt commented Jul 19, 2021

julia> a = randn(3,3)
b =3×3 Matrix{Float64}:
 ra  0.0237017   1.62282  -1.28889
 -0.186575   -2.00309   0.502572
 -0.413123   -1.99718  -0.174593

julia> b = randn(3,3)
3×3 Matrix{Float64}:
 -0.40664   1.80043     1.2399
  1.22559  -0.828653    1.38103
  1.40994  -0.0784539  -0.127629

julia> ForwardDiff.derivative(x -> sum(x*a*b), 0.0)
0.0

julia> ForwardDiff.derivative(x -> sum(x*(a*b)), 0.0)
-6.900806270529478
@antoine-levitt
Copy link
Contributor Author

This is present on 1.7beta and master, but not on 1.6

@antoine-levitt
Copy link
Contributor Author

@mcabbott shot in the dark, is this related to JuliaLang/julia#37898?

@mcabbott
Copy link
Member

That's not good! Yes it's caused by the PR, but I don't know if that's where the bug is. This becomes:

julia> ForwardDiff.derivative(x -> sum(LinearAlgebra.mat_mat_scalar(a, b, x)), 0.0)
0.0

julia> ForwardDiff.derivative(x -> sum(LinearAlgebra.mat_mat_scalar(a, b, x)), 2.0)
0.15095195538098594

julia> ForwardDiff.derivative(0.0) do x
         c = similar(a, typeof(@show x))
         mul!(c, a, b, x, false)
         @show sum(c)
       end
x = Dual{ForwardDiff.Tag{var"#27#28", Float64}}(0.0,1.0)
sum(c) = Dual{ForwardDiff.Tag{var"#27#28", Float64}}(0.0,0.0)
0.0

The explicit mul! form gives zero on Julia 1.5 too. It calls generic_matmatmul!(C, 'N', 'N', A, B, MulAddMul(alpha, beta)) which certainly sounds like it should work. But perhaps it's testing alpha==0, in which case this is one more bug which would be fixed by #481 .

@antoine-levitt
Copy link
Contributor Author

That makes sense. So it's just missing somebody to merge #481?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants