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

Update NLPModelsJuMP for MathOptInterface #37

Closed
wants to merge 6 commits into from

Conversation

amontoison
Copy link
Member

@amontoison amontoison commented Feb 14, 2020

Interface between JuMP models (only composed of @NLobjective and @NLconstraint) and NLPModels.
#22


```@example jumpnlp
```julia
Copy link
Member

Choose a reason for hiding this comment

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

Does this work? @example jumpnlp was supposed to keep the previous environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but the code is not run. It's what I wanted because OptimizationProblems.jl is not working with JuMP 0.20.

I tested locally with an updated version of OptimizationProblems.jl and it works.
amontoison/OptimizationProblems.jl@cf02b59

Copy link
Member Author

Choose a reason for hiding this comment

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

I modified problems such that we have only @NLconstraint, @NLobjective and @NLexpression, it's why I didn't open a pull request.

Otherwise all models work without modifications on JuMP 0.20, we only need to replace setvalue by set_start_value to remove warnings.

return Jtv
end

# Uncomment when :JacVec becomes available in MOI.
Copy link
Member

Choose a reason for hiding this comment

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

Is this still an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, JacVec is still an unsupported feature...

@abelsiqueira
Copy link
Member

I noticed you removed NLS models. Can they be remade?
Also, this needs the latest NLPModels, right?

@amontoison
Copy link
Member Author

amontoison commented Feb 14, 2020

I think it can be remade but at which cost.... They completely changed how linear / quadratic terms are treated inside MOI. If we find a way to parse them, we should build ourself the jacobian , hessian...

With MPB everything was accessible through an MPB.NLPEvaluator (linear or non linear objective / constraints) and a quadratic term was a non linear term. With MOI they made a distinction between a quadratic term a non linear term and only restrictive non linear objective / constraints are accessible through MOI.NLPEvaluator.

I update the Project.toml to use the last release of NLPModels. I didn't test with previous version.

@abelsiqueira
Copy link
Member

You can't get the expressions anymore? MPBNLS was essentially creating constraints for an internal model using expressions.

@abelsiqueira
Copy link
Member

I update the Project.toml to use the last release of NLPModels. I didn't test with previous version.

I meant the master version, because test/problems is not in the latest released version.

@amontoison
Copy link
Member Author

No It doesn't need the masterversion of NLPModels.
Problems are in test/problems folder for NLPModelJuMP and test folder for NLPModel.

module NLPModelsJuMP

include("nlsmpb_model.jl")
Copy link
Member

Choose a reason for hiding this comment

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

Let's comment this out instead of deleting it.


nvar = num_variables(jmodel)
x0 = zeros(nvar)
lvar = []
Copy link
Member

Choose a reason for hiding this comment

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

Maybe initialize this to a Float64 array of -Inf for consistency with x0. Same for uvar.

src/moi_nlp_model.jl Show resolved Hide resolved
src/moi_nlp_model.jl Show resolved Hide resolved

# Uncomment when :JacVec becomes available in MOI.
#
# function NLPModels.jprod(nlp :: MathOptNLPModel, x :: AbstractVector, v :: AbstractVector)
Copy link
Member

Choose a reason for hiding this comment

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

Should disappear.

# return jv
# end

# function NLPModels.jtprod(nlp :: MathOptNLPModel, x :: AbstractVector, v :: AbstractVector)
Copy link
Member

Choose a reason for hiding this comment

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

Should go.

src/moi_nlp_model.jl Show resolved Hide resolved
setvalue(x[1], -10)
setvalue(x[2], 10)
set_start_value(x[1], -10.0)
set_start_value(x[2], 10.0)
Copy link
Member

Choose a reason for hiding this comment

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

At other places, you left Ints as initial value. If they are accepted, I think 10 is clearer than 10.0.

@amontoison
Copy link
Member Author

amontoison commented Feb 15, 2020

I didn't removed jac, jac_structure and hess_structure functions because their implementations for an MathOptNLPModel are more optimized than the generic ones for an AbstractNLPModel.

The main reason is that jrows, jcols, hrows and hcols are stored inside the model. jac_structure/hess_structure don't need to call jac_structure!/hess_structure! and jac don't need to call jac_structure.

.travis.yml Outdated
addons:
apt_packages:
- gfortran # For Ipopt

Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be part of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see that you just add it in #32. It should disappear after a rebase.

appveyor.yml Outdated
@@ -2,6 +2,8 @@ environment:
matrix:
- julia_version: 1.0
- julia_version: 1.1
- julia_version: 1.2
- julia_version: 1.3
Copy link
Member

Choose a reason for hiding this comment

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

This is already in #32.

dpo added a commit that referenced this pull request Feb 15, 2020
dpo added a commit that referenced this pull request Feb 15, 2020
dpo added a commit that referenced this pull request Feb 15, 2020
dpo added a commit that referenced this pull request Feb 15, 2020
dpo added a commit that referenced this pull request Feb 16, 2020
@dpo dpo changed the title Update NLPModelJuMP for MathOptInterface Update NLPModelsJuMP for MathOptInterface Feb 17, 2020
@amontoison
Copy link
Member Author

I close this pull request, #38 includes MathOptNLPModel and MathOptNLSModel.

@amontoison amontoison closed this Feb 18, 2020
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