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

RFC: import NLP interface from MathProgBase with minor modifications #202

Merged
merged 6 commits into from
Feb 11, 2018

Conversation

mlubin
Copy link
Member

@mlubin mlubin commented Jan 29, 2018

Given that there's been no movement on designing a nice MOI interface for NLP, I think the practical step forward is to import the interface from MPB basically as-is so that we can get NLP solvers working under JuMP/MOI.

In the PR I add an NLPBlock attribute that's used to store a block of NLP constraints and optionally an objective. If a user writes down @NLobjective in JuMP, the objective will show up in the NLPBlock. Otherwise, it will be set with ObjectiveFunction as in normal MOI. The most significant change on the solver interface side is that, unless the solver wants to force users to use @NLobjective and @NLconstraint entirely, it will need to support linear+quadratic objectives and constraints added through the normal MOI interface. This will be a bit of a pain, but it also allows solvers to support NLP + conic or NLP + complementarity just by supporting the corresponding MOI constraints and an NLPBlock.

I also added the NLPBlockDualStart attribute that finally lets you give a dual starting point.

@odow @ccoffrin @chriscoey @dpo @chkwon @sylvainmouret @abelsiqueira @adowling2 @timholy (If you're not sure why you're CC'd: the next release of JuMP will not support MPB. This API will be the new way JuMP talks to NLP solvers.)

TODO before merging: The interface still needs to be tweaked to account for VectorIndex. I'm thinking of a call to AbstractNLPEvaluator to set the evaluation point, then we can remove the x argument from the evaluation functions. obj_expr and constr_expr would use the variable indices in :(x[i]) as well. The API is now set up so that you provide an ordering of the variable indices in initialize. All other interactions with the AbstractNLPEvaluator are based on indices 1, ..., n. (Maybe obj_expr and constr_expr should be an exception?)

There's also a bit of room for bikeshedding the names. (e.g., eval_f to eval_objective?)

@chkwon
Copy link

chkwon commented Jan 29, 2018

I am curious about how NLP + complementarity will be supported. Is it going to be some reformulations, I guess?

@ccoffrin
Copy link

@kaarthiksundar, @jac0320, @Wikunia, all of our JuMP solvers will need to be updated to this new NLP API to support JuMP v0.19. Now is the time to review this slightly revised NLP API and share any insights/suggestions you might have. Also a good time to bring up minor feature requests that would be helpful to our solvers.

CC @rb004f, @harshangrjn, @ad2476, @hhijazi

@ccoffrin
Copy link

One thing we noticed recently in MPB is that KNITRO has a freemodel! method but IPOPT does not. I am thinking in MOI this issue will go away, but good to double check.

@mlubin
Copy link
Member Author

mlubin commented Jan 30, 2018

@chkwon, this API provides a way to specify an NLP with complementarity constraints in JuMP and send it directly to a solver. It's up to the solver to decide if/how to support them. The syntax would look something like:

... build nonlinear model
@constraint(m, [x,y] in Complements())

where you'd define Complements() as an MOI set. If you wanted to solve this problem with Ipopt you'd still need a wrapper around Ipopt, but not a JuMP extension.

@ccoffrin, the equivalent of MPB.freemodel! is MOI.free!. It's usually only useful for telling commercial solvers to release licenses, which is why I guess nobody took the effort to implement it for Ipopt. But in general for MOI we don't want method_exists to be a normal way to check for functionality as it is with MPB.

@blegat
Copy link
Member

blegat commented Feb 1, 2018

Why don't we do something like

abstract type AbstractScalarNLPEvaluator <: AbstractNLPEvaluator end
abstract type AbstractVectorNLPEvaluator <: AbstractNLPEvaluator end
struct ScalarNonLinearFunction
    evaluator::AbstractScalarNLPEvaluator
end
struct VectorNonLinearFunction
    evaluator::AbstractVectorNLPEvaluator
end

The former has a gradient and can be used for the objective while the latter has a hessian and can be used for the constraints.

eval(::AbstractNLPEvaluator)
eval_grad(::AbstractScalarNLPEvaluator)
eval_jac(::AbstractVectorNLPEvaluator)
expr(::AbstractScalarNLPEvaluator)
expr(::AbstractVectorNLPEvaluator, i)

The constraints can then be added with addconstraint!, ... and the objective can be added with set! as usual. It seems we are missing the AbstractVectorSetequivalent ofInterval` though.

@mlubin
Copy link
Member Author

mlubin commented Feb 1, 2018

@blegat, that's heading in the opposite direction from where I'd like to the nonlinear interface to go. An ideal nonlinear interface would pass only expressions to the solver, and solvers would call out to AD libraries to compute derivatives if they want to. The only time evaluator callbacks are needed are for user-defined functions. Given that nobody's taken the lead on this, we're porting over the MPB interface with no big structural changes.

@mlubin
Copy link
Member Author

mlubin commented Feb 2, 2018

To give a bit more reasoning for why I'd like to do that, removing the AD from the JuMP side and having expressions as first-class MOI objects would make it easier to write solvers that manipulate expressions. Currently there's no super fast path to do this since it involves converting from JuMP's internal expression representation to Expr objects, manipulating them, then adding them back through JuMP.addNLconstraint, which I believe is what some of the LANL solvers do.

@chriscoey
Copy link
Contributor

I agree with @mlubin (and it's why I wanted to delay porting over the old NLP interface to MOI). I plan to work on solvers that use and manipulate the expressions and don't care about the derivatives. It makes sense for solvers to call AD if they rely on it, and not call it if they don't.

@ccoffrin
Copy link

ccoffrin commented Feb 2, 2018 via email

@blegat
Copy link
Member

blegat commented Feb 2, 2018

I agree with you. The proposal in my comment, just like this PR, is meant to be transitional and to be replaced by expressions (e.g. by replacing the evaluator field by the expression).

@mlubin
Copy link
Member Author

mlubin commented Feb 2, 2018

@blegat your proposal requires a substantial restructuring of JuMP's AD

@blegat
Copy link
Member

blegat commented Feb 2, 2018

Why would it require a different structure than yours ? JuMP could still put all nlp constraints in a single vector function so both proposal require the same structure in JuMP. Instead of having a single evaluator and dispatching between objective and constraints with the suffix of the functions, there is 2 evaluators.

One advantage of having nlp functions as MOI AbstractFunction is that it requires no change to MOIU.

@mlubin
Copy link
Member Author

mlubin commented Feb 2, 2018

What are the changes needed in MOIU to support this? I see the variable mapping as one point that may need to be wrapped, but that needs to happen regardless of if NLP data is passed as an attribute or as constraints and objectives.

@blegat
Copy link
Member

blegat commented Feb 3, 2018

You are right, I forgot about variable mapping. As it is an attribute, there might be no change needed indeed.
Once we use the expression based approach, will we use an attribute or the constraint+objective approach ?

@mlubin
Copy link
Member Author

mlubin commented Feb 3, 2018

Once we use the expression based approach, will we use an attribute or the constraint+objective approach ?

Definitely constraint+objective

@mlubin
Copy link
Member Author

mlubin commented Feb 5, 2018

I did another round of renaming towards more explicit names. I'll merge soon barring additional comments.

src/nlp.jl Outdated
All subsequent references to the vector ``x`` follow this index mapping; the
`i`th index of ``x`` corresponds to `variable_order[i]`.
"""
function initialize end
Copy link
Member

Choose a reason for hiding this comment

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

Pedantic proposal: should this perhaps be initialize!?

@mlubin mlubin merged commit 6e0e666 into master Feb 11, 2018
@mlubin mlubin deleted the ml/nlp branch February 11, 2018 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants