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 #38

Merged
merged 14 commits into from
Mar 8, 2020

Conversation

amontoison
Copy link
Member

@amontoison amontoison commented Feb 18, 2020

close #22
close #41

Copy link
Member

@abelsiqueira abelsiqueira left a comment

Choose a reason for hiding this comment

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

Very small changes. I haven't checked the API changes, I'm trusting consistency here.

Project.toml Outdated Show resolved Hide resolved
docs/Project.toml Outdated Show resolved Hide resolved
@@ -2,6 +2,7 @@ using Documenter, NLPModelsJuMP

makedocs(
modules = [NLPModelsJuMP],
checkdocs = :exports,
Copy link
Member

Choose a reason for hiding this comment

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

What does this do?

Copy link
Member Author

@amontoison amontoison Feb 18, 2020

Choose a reason for hiding this comment

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

Without checkdocs = :exports, documenter says that the documentation of replace! is missing. And with strict = true the build of the documentation is failing.

@coveralls
Copy link

coveralls commented Feb 18, 2020

Coverage Status

Coverage increased (+23.3%) to 100.0% when pulling ebe5356 on amontoison:moi into 2bca1e9 on JuliaSmoothOptimizers:master.

Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

That's a lot of changes at once. Here's a first round of comments. Many thanks for all this!!!

docs/src/tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorial.md Show resolved Hide resolved
docs/src/tutorial.md Show resolved Hide resolved
@@ -1,9 +1,7 @@
__precompile__()
Copy link
Member

Choose a reason for hiding this comment

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

Does precompilation fail?

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 thought that precompilation was done by default with the last versions of Julia, is it not the case?

src/moi_nlp_model.jl Outdated Show resolved Hide resolved
src/moi_nlp_model.jl Outdated Show resolved Hide resolved
src/moi_nlp_model.jl Outdated Show resolved Hide resolved
src/moi_nlp_model.jl Outdated Show resolved Hide resolved
src/moi_nlp_model.jl Outdated Show resolved Hide resolved
src/moi_nlp_model.jl Show resolved Hide resolved
src/moi_nlp_model.jl Outdated Show resolved Hide resolved
@amontoison
Copy link
Member Author

amontoison commented Mar 2, 2020

@abelsiqueira @dpo
I updated MathOptNLPModel for NLPModel v0.12 :

  • jac and hess functions removed
  • jprod!(nlp, x, v, Jv) and jtprod!(nlp, x, v, Jtv) updated
  • hess_structure! and jac_structure! modified because jrows, jcols, jvals, hrows, hcols and hvals are not stored anymore in the model
  • jprod!(nlp, x, rows, cols, v, Jv) and jtprod!(nlp, x, rows, cols, v, Jtv) functions added
  • :ExprGraph removed (should avoid OufOfMemory error on power OptimizationProblems.jl#38)

I also updated MathOptNLSModel :

  • jac_residual, jac, hess_residual, hess and jth_hess_residuals functions removed
  • hess_structure_residual!, jac_structure_residual!, hess_structure! and jac_structure! modified because Fjrows, Fjcols, Fjvals, Fhrows, Fhcols, Fhvals, cjrows, cjcols, cjvals, chrows, chcols, chvals are not stored anymore in the model
  • jprod_residual!(nlp, x, v, Jv), jtprod_residual!(nlp, x, v, Jtv), jprod!(nlp, x, v, Jv) and jtprod!(nlp, x, v, Jtv) updated
  • :ExprGraph only called when required

Do I forget something or not?

Copy link
Member

@abelsiqueira abelsiqueira left a comment

Choose a reason for hiding this comment

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

I think that's it.

Project.toml Outdated
MathProgBase = "0.7.6"
NLPModels = "0.11"
julia = "^1.0.0"
JuMP = "0.21"
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't it work with JuMP > 0.19?

Copy link
Member Author

Choose a reason for hiding this comment

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

It works with JuMP 0.19, 0.20 and 0.21. But you asked me to leave only one version of JuMP.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I wasn't clear. I meant on the docs/Project.toml to just leave the latest version. The main Project.toml should handle more versions, to avoid conflicts.

@@ -4,9 +4,6 @@ task:
name: FreeBSD
env:
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.

You can also add 1.4 now.

Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

We need a new release of NLPModels first, right? @abelsiqueira

src/moi_nlp_model.jl Outdated Show resolved Hide resolved
src/moi_nls_model.jl Outdated Show resolved Hide resolved
src/moi_nls_model.jl Outdated Show resolved Hide resolved
@abelsiqueira
Copy link
Member

Looks great, thank you!

We need a new release of NLPModels first, right? @abelsiqueira

Yes, but Alexis asked for JuliaSmoothOptimizers/NLPModels.jl#257 before 0.12.0

@abelsiqueira
Copy link
Member

@dpo dpo merged commit e1ec5e9 into JuliaSmoothOptimizers:master Mar 8, 2020
@dpo
Copy link
Member

dpo commented Mar 8, 2020

Hooray!!!

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.

Update NLPModelsJuMP to use MathOptInterface
4 participants