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

switch argument order for gradient and hessian evaluations? #156

Closed
ahwillia opened this Issue Dec 30, 2015 · 3 comments

Comments

Projects
None yet
4 participants
@ahwillia
Contributor

ahwillia commented Dec 30, 2015

It would be nice if the inputs g!, fg!, h!, etc. matched what ForwardDiff.jl returns when you call gradient, hessian, etc. Specifically, the storage/output vector is the first argument in ForwardDiff.

g! = ForwardDiff.gradient(f, mutates=true)
# compute gradient at x
g!(storage, x)

Meanwhile Optim expects:

g!(x, storage)

I think having the mutated argument come first makes sense as a convention. However, this is a small detail, and it will break a lot of code. So maybe it is best to keep things the way they are, but wanted to bring up this possibility.

@KristofferC

This comment has been minimized.

Contributor

KristofferC commented Dec 30, 2015

+1 from me.

As it is right now you need to do quite some juggling with function argument to make it work with ForwardDiff, see for example https://github.com/EconForge/NLsolve.jl/blob/master/src/autodiff.jl

The problem is that many of the julia optimization/solver packages are consistent on having the mutated input last, i.e.:

https://github.com/JuliaOpt/NLopt.jl and https://github.com/EconForge/NLsolve.jl.

so maybe it is a too disruptive of a change...

@johnmyleswhite

This comment has been minimized.

Contributor

johnmyleswhite commented Dec 30, 2015

I'm happy to do this after we make some other breaking changes. I've always wanted to move away from the current API.

@pkofod

This comment has been minimized.

Collaborator

pkofod commented Feb 1, 2016

+1 for having the mutated object first. I think this is what most Julia packages/bases uses as well (push!, plot!, ...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment