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

Adjoint for ForwardDiff.jacobian #968

Merged
merged 4 commits into from
May 15, 2021
Merged

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented May 8, 2021

This adds one-line adjoint definitions for ForwardDiff.gradient and ForwardDiff.jacobian, using forwarddiff. This is a simpler version of what was proposed in #769.

Perhaps ideally, calling Zygote over ForwardDiff would actually do reverse mode over forward mode. The proposal in #769 was in fact to turn that into forward over reverse, while this PR is forward over forward. Maybe there are cases where one is much better than the other?

But it doesn't work at all right now. Errors involved are, BTW, these:

Mutating arrays is not supported

Need an adjoint for constructor ForwardDiff.Partials{1, Float64}. Gradient is of type Vector{ForwardDiff.Dual{ForwardDiff.Tag{var"#66#67", Float64}, Float64, 1}}

Closes #305 (gradient of a function containing Zygote.forward_jacobian, replace with ForwardDiff.jacobian) and maybe #953 (gradient of a function containing jacobian, could perhaps use ForwardDiff.jacobian).

@CarloLucibello
Copy link
Member

this seems a good temporary solution until we have reverse over forward or forward over reverse. Maybe leave a comment over the new adjoints explaining this is doing forward over forward and link related discussions? I think we should merge then

src/lib/forward.jl Outdated Show resolved Hide resolved
@mcabbott
Copy link
Member Author

Done. I also added derivative and hessian, not sure why I omitted them at first.

Note BTW the new failure on master, I think this is caused by JuliaLang/julia#40745

@CarloLucibello
Copy link
Member

Nightly failed addressed in FluxML/IRTools.jl#86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
second order zygote over zygote, or otherwise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot take gradient of function of jacobian
2 participants