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

Add jacobian function #747

Closed
wants to merge 6 commits into from
Closed

Conversation

ChrisRackauckas
Copy link
Member

Fixes #98

src/lib/utils.jl Outdated Show resolved Hide resolved
@CarloLucibello
Copy link
Member

needs a test and a mention in the docs

Co-authored-by: Carlo Lucibello <[email protected]>
Copy link
Member

@mcabbott mcabbott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be good to have, but shouldn't it behave like gradient and return a 1-tuple for one argument? Then the extension to several arguments need not break anything.

src/lib/utils.jl Outdated Show resolved Hide resolved
src/lib/utils.jl Outdated Show resolved Hide resolved
src/lib/utils.jl Outdated Show resolved Hide resolved
@cossio
Copy link
Contributor

cossio commented Sep 15, 2020

Bump? Would be nice to have.

@MasonProtter
Copy link
Contributor

If we split out OneHotVector from Flux.jl, we could have this simpler implementation that's sometimes more performant and has less weird edge cases:

using Flux: OneHotVector

"""
jacobian(f,x)
Construct the Jacobian of `f` where `x` is a real-valued array
and `f(x)` is also a real-valued array.
"""
function jacobian(f, x)
    y, ∂f = Zygote.pullback(f,x)
    n, m  = length(x), length(y)
    mapreduce(i -> ∂f(OneHotVector(i, m)), hcat, 1:m)
end

ChrisRackauckas and others added 2 commits September 15, 2020 18:31
Co-authored-by: Michael Abbott <[email protected]>
Co-authored-by: Michael Abbott <[email protected]>
Copy link
Member

@DhairyaLGandhi DhairyaLGandhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but needs tests

src/lib/utils.jl Outdated Show resolved Hide resolved
@DhairyaLGandhi
Copy link
Member

Also probably something that should hook into the forward mode

@ChrisRackauckas
Copy link
Member Author

Forward mode Jacobian should be kept as a separate call: which one is better depends on the application so both are required. What this should really connect to is SparseDiffTools for sparse Jacobians, but what will happen there is that SparseDiffTools will have a... more intense version that handles all of that extra machinery.

@DhairyaLGandhi
Copy link
Member

This is due to it detecting sparsity patterns I assume?

src/lib/utils.jl Outdated Show resolved Hide resolved
Co-authored-by: Mason Protter <[email protected]>
@ChrisRackauckas
Copy link
Member Author

This is due to it detecting sparsity patterns I assume?

Well this could be made to accept a color vector even without SparseDiffTools.

src/lib/utils.jl Outdated Show resolved Hide resolved
Co-authored-by: Michael Abbott <[email protected]>
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 this pull request may close these issues.

Jacobian and Hessian
6 participants