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

Traits interface? #87

Open
timholy opened this issue Oct 14, 2015 · 10 comments
Open

Traits interface? #87

timholy opened this issue Oct 14, 2015 · 10 comments
Labels

Comments

@timholy
Copy link
Contributor

timholy commented Oct 14, 2015

Normally I'd just hack this up and submit a PR, but I'm feeling short of time so will first float a suggestion and see what people think.

For nonlinear optimization, the API is good and flexible, but to me it still feels like there's more boilerplate required from the user than would be ideal. I'm wondering whether it would be better to define a traits-based API, e.g.,

abstract NLConstraintsType
immutable NLConstraintsNone <: NLConstraintsType end
immutable NLConstraintsJac <: NLConstraintsType end
immutable NLConstraintsJacProd <: NLConstraintsType end

nlconstraints(d::AbstractNLPEvaluator) = NLConstraintsJac()   # default value

# fallback definition, so the user doesn't have to define this
SolverInterface.eval_g(d, g, x) = SolverInterface.eval_g(nlconstraints(d), d, g, x)
SolverInterface.eval_g(::NLConstraintsNone, d, g, x) = nothing

# do the same thing for jac_structure, eval_jac_g
...

so the user can define

MathProgBase.nlconstraints(::MyEvaluator) = NLConstraintsNone()

and then not have to worry about any of the g-related functions. Same strategy for the various Hessian options, etc. These would essentially replace initialize and features_available, at least as far as users are concerned, and as illustrated the more granular design reduces the need for boilerplate stub functions.

In my own code I do this via inheritance, e.g.,

abstract BoundsOnly <: SolverInterface.AbstractNLPEvaluator
SolverInterface.eval_g(::BoundsOnly, g, x) = nothing

but the traits-based mechanism is far better because it allows you to mix-and-match traits arbitrarily.

@mlubin
Copy link
Member

mlubin commented Oct 14, 2015

The user experience of implementing the NLP API wasn't really prioritized in the design, so I agree that there's room for improvement. Could you expand on how this replaces initialize? There's an important two-way communication that takes place there; JuMP only spends time setting up data structures for hessian evaluations if the solver wants hessians, so whether or not JuMP provides hessians is a run-time property. Same for jacobians.

@timholy
Copy link
Contributor Author

timholy commented Oct 14, 2015

I was basing my statement off of implementations like this, for which the suitability of a given requested feature could be decided by calling the appropriate trait-function. For example, if hessiankind(d) == HessianNone(), then don't set up Hessian info.

@mlubin
Copy link
Member

mlubin commented Oct 14, 2015

If the signature is hessiankind(::AbstractNLPEvaluator) then this is something implemented by the user. (By user I mean both user-defined implementations and JuMP.) At what point would the solver communicate to the user whether or not hessians are needed?

@timholy
Copy link
Contributor Author

timholy commented Oct 14, 2015

I guess what I'm saying is that you can still have an internal initialize function, but the user doesn't have to bother writing it herself. The implementation is

function initialize(d, requested_features)
    ...stuff...
    if feat == :Hess && !isa(hessiankind(d), HessianExplicit)
        error("You must supply an explicit hessian to use this solver")
    end
    ...stuff...
end

EDIT: once the user sets up the proper traits for his/her type, everything else is free except for the implementations of the methods that s/he wants to support.

@mlubin
Copy link
Member

mlubin commented Oct 14, 2015

I see how that could work. There are a lot of subtle implicit design points but should be tractable. How breaking is this, if at all?

@joehuchette
Copy link
Member

We've been using applicable calls to do something like this throughout the JuMP codebase for linear problems. Perhaps we should move to a traits-based interface there as well.

@mlubin
Copy link
Member

mlubin commented Oct 14, 2015

The advantage of traits over introspection (applicable), IIRC is that they're usually type stable and easier to optimize. These benefits generally don't apply here, so if we switch there should be another good reason (e.g. decreasing complexity).

@joehuchette
Copy link
Member

Yes, the primary gain would be unified way for the solver to communicate what features they support. Plus, doing method lookup feels brittle (what if you define a stub method?).

@timholy
Copy link
Contributor Author

timholy commented Oct 14, 2015

How breaking is this, if at all?

Not certain, but my inclination is to say that it's not. Users of this interface may have written methods that, because they're more specific than your fallbacks, should take precedence.

@mlubin
Copy link
Member

mlubin commented Jun 9, 2017

It's worth rethinking about traits more generally, especially for jump-dev/JuMP.jl#83

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

No branches or pull requests

4 participants