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

abstract DifferentiableFunction #128

Closed
wants to merge 2 commits into from

Conversation

alyst
Copy link
Contributor

@alyst alyst commented Jul 8, 2015

This is related to #53, #94, #102 etc.
With the current implementation of DifferentiableFunction it's not very convenient to implement functions that require external data for evaluation. It always ends up writing something like

df = DifferentiableFunction(x -> real_func(x, data) .... )

before every optimizer invocation.

This PR turns DifferentiableFunction into an abstract type, and calls like df.f(x) are supposed to be replaced by evalf(df, x).
The old DifferentiableFunction is renamed into SimpleDifferentiableFunction. Since SimpleDifferentiableFunction is used when one wants to convert 3 functions into DifferentiableFunction object, the current Optim code does not require any modifications.

alyst added 2 commits July 8, 2015 13:47
- df.f(x) syntax in the long run should be replaced by evalf(df, x)
- SimpleDifferentiableFunction is the Function-based implementation
  that gets used by default and is compatible with the df.f(x) syntax
  used for the old DifferentiableFunction
- df.f(x) is replaced in linesearch algorithms (interpolating linesearch
  has come minor optimizations of calling evalfg!() instead of separate
  calls to f() and g!())
- TwiceDifferentiableFunction also made abstract
@johnmyleswhite
Copy link
Contributor

I'm not really sold on this change. In general, I think we should exploit attempts to optimize closures in Base Julia rather than try to work around it.

I also would like to see us move away from the DifferentiableFunction type someday and use normal function types.

@alyst
Copy link
Contributor Author

alyst commented Jul 15, 2015

It might be regarded as a closure performance workaround, but the intention is to allow callable objects be used with Optim package.
I thought subclassing DifferentiableFunction would be also a natural way to specify how the gradient/hessian should be calculated, i.e. Autodiff, FiniteDifference (subtype allows specifying whether we want left, center or right) etc.

@johnmyleswhite
Copy link
Contributor

I'm hoping to move in the opposite direction as we redesign the API, so closing out.

@alyst
Copy link
Contributor Author

alyst commented Oct 13, 2015

Is there some document that outlines the new design of Optim?
I would like to understand better how it fits the use cases I have in mind.

@johnmyleswhite
Copy link
Contributor

In the long term, I'd like to move towards an API that takes in arbitrary callables and uses only the number of arguments and the types of the trailing arguments to do dispatch.

For a concrete example, something like optimize(f, x0::Vector, ::Type{LBFGS}) vs optimize(f, g!, x0::Vector, ::Type{LBFGS}) would be used instead of the differentiable function machinery. We would have implicit contracts on f and g! rather than trying to force them via DifferentiableFunction.

We could also adopt a more complex API like that used by NLopt. For now, I mostly want to remove the gap between the optimize wrapper and the inner methods that get called because the optimize wrapper gets tasked with too much moving around of arguments.

@timholy
Copy link
Contributor

timholy commented Jan 26, 2016

In the long term, I'd like to move towards an API that takes in arbitrary callables and uses only the number of arguments and the types of the trailing arguments to do dispatch.

I think this PR implements the very API you're talking about. evalfg!(df, x, gr) is a much more general interface (one that relies on dispatch) than df.fg!(x, gr). #163 provides another example of where you need to be able to pass in (and dispatch on) something that's not one of the "core" numerical types.

Reopen?

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.

3 participants