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

Error with SolverBenchmark #121

Open
MaxenceGollier opened this issue Jul 22, 2024 · 8 comments
Open

Error with SolverBenchmark #121

MaxenceGollier opened this issue Jul 22, 2024 · 8 comments

Comments

@MaxenceGollier
Copy link

I am trying to benchmark solvers on a Diagonal Quasi-Newton Model. I get two issues:

  1. I get type DiagonalQNModel has no field counters
  2. When giving a Quasi-Newton Model to Ipopt, I get MethodError : no method matching hess_structure!(::NLPModelsModifiers.DiagonalQNModel{...},...)

Here is an example

using BenchmarkTools, Dates, LinearAlgebra, JLD2

using Percival, NLPModels, NLPModelsIpopt, SolverBenchmark, SolverCore, NLPModelsModifiers
using OptimizationProblems, ADNLPModels, OptimizationProblems.ADNLPProblems

meta = OptimizationProblems.meta
problem_names = meta[(meta.has_equalities_only.==1).&(meta.has_bounds.==0).&(meta.has_fixed_variables.==0).&(meta.variable_nvar.==0), :name]
problem_list = (SpectralGradientModel(eval(Meta.parse(name))(), σ=1.0) for name in problem_names)

solvers = Dict(
  :ipopt =>
    nlp -> ipopt(
      nlp,
    ),
  :percival =>
    nlp -> percival(
      nlp,
    ),
)

stats = bmark_solvers(solvers, problem_list)

There is no reason for not having a hess_structure! method at least for the DiagonalQNModel type because it is trivial. I can also add the counters as a field of the different structures if you think it is appropriate.

@dpo
Copy link
Member

dpo commented Jul 22, 2024

I probably didn’t implement counters and structure because diagonal QN operators are only used in the nonsmooth solvers at this point. You are right that they should be added.

@tmigot
Copy link
Member

tmigot commented Jul 23, 2024

@MaxenceGollier thanks for the issue.
For the other quasi-Newton models, this is the line that adds the counters field:

@default_counters QuasiNewtonModel model

It is a macro that redirects the counters field to the original nlp.

I agree with @dpo it makes sense to have Hessian functions implemented for AbstractDiagonalQNModel

@tmigot
Copy link
Member

tmigot commented Jul 23, 2024

@MaxenceGollier would you have some time to open two PRs for these two issues?

@MaxenceGollier
Copy link
Author

@MaxenceGollier thanks for the issue. For the other quasi-Newton models, this is the line that adds the counters field:

@default_counters QuasiNewtonModel model

It is a macro that redirects the counters field to the original nlp.

I agree with @dpo it makes sense to have Hessian functions implemented for AbstractDiagonalQNModel

I don't get it, This macro does not seem to work as if nlp is type QuasiNewtonNLP, then nlp.counters doesn't exist, what should I modify ? Just adding the field is not correct ? (as I did in #122)

@MaxenceGollier
Copy link
Author

I have an issue for adding the hessian structure with the field nnzh of nlp.meta, I opened an issue in NLPModels.jl

@MaxenceGollier
Copy link
Author

@MaxenceGollier thanks for the issue. For the other quasi-Newton models, this is the line that adds the counters field:

@default_counters QuasiNewtonModel model

It is a macro that redirects the counters field to the original nlp.

I agree with @dpo it makes sense to have Hessian functions implemented for AbstractDiagonalQNModel

Actually, wouldn't this macro be more appropriate ?

macro add_counters(Model, inner)
  :($Model.counters = $Model.$inner.counters)
end

since we want to access to the field counters with AbstractDiagonalQNModel type

@tmigot
Copy link
Member

tmigot commented Jul 24, 2024

Actually, the @default_counters macro does more than just accessing the inner counters see
https://github.com/JuliaSmoothOptimizers/NLPModels.jl/blob/b8454fbb8bb1399c1c88b7597db88756536e501f/src/nlp/utils.jl#L119 That's why I am really trying to make it work.

@tmigot
Copy link
Member

tmigot commented Jul 24, 2024

Just for reference, the meta discussion is connected to #29

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

No branches or pull requests

3 participants