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

fix bug in MOI impl when quadratic term has param #379

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

guimarqu
Copy link
Contributor

Hi,

I wanted to try the solver on a problem but optimization of my NLP threw an error.
I could reproduce with this example

# Example 3
using JuMP, MadNLP, MathOptInterface
const MOI = MathOptInterface

model = Model(MadNLP.Optimizer)

@variable(model, 0 <= x <= 2)
@variable(model, 0 <= y <= 2)
@variable(model, a in MOI.Parameter(3.0))
@variable(model, b in MOI.Parameter(3.0))

@constraint(model, cstr3, a*x^2 + b*y^2 <= 1)
@objective(model, Max, x + y)
optimize!(model)

There are two bugs :

The first one is easy to fix :

ERROR: type Optimizer has no field inner
Stacktrace:
  [1] setproperty!(x::MadNLPMOI.Optimizer, f::Symbol, v::Nothing)
    @ Base ./Base.jl:39
  [2] add_constrained_variable(model::MadNLPMOI.Optimizer, set::MathOptInterface.Parameter{Float64})
    @ MadNLPMOI ~/.julia/packages/MadNLP/66k4O/ext/MadNLPMOI/MadNLPMOI.jl:171
  [3] add_constrained_variable(b::MathOptInterface.Bridges.LazyBridgeOptimizer{…}, set::MathOptInterface.Parameter{…})
    @ MathOptInterface.Bridges ~/.julia/packages/MathOptInterface/gLl4d/src/Bridges/bridge_optimizer.jl:2139
  [4] _add_variable_with_domain(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{…}, src::MathOptInterface.Utilities.UniversalFallback{…}, index_map::MathOptInterface.Utilities.IndexMap, f::Vector{…}, ci::MathOptInterface.ConstraintIndex{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:504
  [5] _copy_variables_with_set(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{…}, src::MathOptInterface.Utilities.UniversalFallback{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:548
  [6] default_copy_to(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{…}, src::MathOptInterface.Utilities.UniversalFallback{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:386
  [7] copy_to
    @ ~/.julia/packages/MathOptInterface/gLl4d/src/Bridges/bridge_optimizer.jl:442 [inlined]
  [8] optimize!
    @ ~/.julia/packages/MathOptInterface/gLl4d/src/MathOptInterface.jl:121 [inlined]
  [9] optimize!(m::MathOptInterface.Utilities.CachingOptimizer{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/cachingoptimizer.jl:321
 [10] optimize!(model::Model; ignore_optimize_hook::Bool, _differentiation_backend::MathOptInterface.Nonlinear.SparseReverseMode, kwargs::@Kwargs{})
    @ JuMP ~/.julia/packages/JuMP/FEKLB/src/optimizer_interface.jl:595
 [11] optimize!(model::Model)
    @ JuMP ~/.julia/packages/JuMP/FEKLB/src/optimizer_interface.jl:546
 [12] top-level scope
    @ REPL[30]:1
Some type information was truncated. Use `show(err)` to see complete types.

The second one is a missing method for function _is_parameter :

ERROR: MethodError: no method matching _is_parameter(::MathOptInterface.ScalarQuadraticTerm{Float64})

Closest candidates are:
  _is_parameter(::MathOptInterface.VariableIndex)
   @ MadNLPMOI ~/.julia/packages/MadNLP/66k4O/ext/MadNLPMOI/MadNLPMOI.jl:12
  _is_parameter(::MathOptInterface.ScalarAffineTerm)
   @ MadNLPMOI ~/.julia/packages/MadNLP/66k4O/ext/MadNLPMOI/MadNLPMOI.jl:14

Stacktrace:
  [1] _any
    @ ./reduce.jl:1220 [inlined]
  [2] any
    @ ./reducedim.jl:1020 [inlined]
  [3] _replace_parameters(model::MadNLPMOI.Optimizer, f::MathOptInterface.ScalarQuadraticFunction{Float64})
    @ MadNLPMOI ~/.julia/packages/MadNLP/66k4O/ext/MadNLPMOI/MadNLPMOI.jl:211
  [4] _replace_parameters(model::MadNLPMOI.Optimizer, f::MathOptInterface.ScalarNonlinearFunction) (repeats 2 times)
    @ MadNLPMOI ~/.julia/packages/MadNLP/66k4O/ext/MadNLPMOI/MadNLPMOI.jl:221
  [5] add_constraint(model::MadNLPMOI.Optimizer, f::MathOptInterface.ScalarNonlinearFunction, s::MathOptInterface.LessThan{Float64})
    @ MadNLPMOI ~/.julia/packages/MadNLP/66k4O/ext/MadNLPMOI/MadNLPMOI.jl:471
  [6] _copy_constraints(dest::MadNLPMOI.Optimizer, src::MathOptInterface.Utilities.UniversalFallback{…}, index_map::MathOptInterface.Utilities.IndexMap, index_map_FS::MathOptInterface.Utilities.DoubleDicts.IndexDoubleDictInner{…}, cis_src::Vector{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:180
  [7] _copy_constraints(dest::MadNLPMOI.Optimizer, src::MathOptInterface.Utilities.UniversalFallback{…}, index_map::MathOptInterface.Utilities.IndexMap, cis_src::Vector{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:192
  [8] pass_nonvariable_constraints_fallback(dest::MadNLPMOI.Optimizer, src::MathOptInterface.Utilities.UniversalFallback{…}, index_map::MathOptInterface.Utilities.IndexMap, constraint_types::Vector{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:203
  [9] pass_nonvariable_constraints
    @ ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:229 [inlined]
 [10] pass_nonvariable_constraints(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{…}, src::MathOptInterface.Utilities.UniversalFallback{…}, idxmap::MathOptInterface.Utilities.IndexMap, constraint_types::Vector{…})
    @ MathOptInterface.Bridges ~/.julia/packages/MathOptInterface/gLl4d/src/Bridges/bridge_optimizer.jl:426
 [11] _pass_constraints(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{…}, src::MathOptInterface.Utilities.UniversalFallback{…}, index_map::MathOptInterface.Utilities.IndexMap, variable_constraints_not_added::Vector{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:251
 [12] default_copy_to(dest::MathOptInterface.Bridges.LazyBridgeOptimizer{…}, src::MathOptInterface.Utilities.UniversalFallback{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/copy.jl:393
 [13] copy_to
    @ ~/.julia/packages/MathOptInterface/gLl4d/src/Bridges/bridge_optimizer.jl:442 [inlined]
 [14] optimize!
    @ ~/.julia/packages/MathOptInterface/gLl4d/src/MathOptInterface.jl:121 [inlined]
 [15] optimize!(m::MathOptInterface.Utilities.CachingOptimizer{…})
    @ MathOptInterface.Utilities ~/.julia/packages/MathOptInterface/gLl4d/src/Utilities/cachingoptimizer.jl:321
 [16] optimize!(model::Model; ignore_optimize_hook::Bool, _differentiation_backend::MathOptInterface.Nonlinear.SparseReverseMode, kwargs::@Kwargs{})
    @ JuMP ~/.julia/packages/JuMP/FEKLB/src/optimizer_interface.jl:595
 [17] optimize!(model::Model)
    @ JuMP ~/.julia/packages/JuMP/FEKLB/src/optimizer_interface.jl:546
 [18] top-level scope
    @ REPL[10]:1
Some type information was truncated. Use `show(err)` to see complete types.

I also tried with this additional constraint to make sure I replace all parameters (otherwise automatic diff pkg throws an error about unexisting variable which is in fact a parameter) :

@constraint(model, cstr1, a*x + b^2*y + a*b^2 <= 100)

I don't know where I can put the tests. Maybe it should go into MOI tests ?

@guimarqu guimarqu marked this pull request as ready for review November 14, 2024 13:17
@sshin23
Copy link
Member

sshin23 commented Nov 14, 2024

Thanks for the contribution @guimarqu! Do you mind including your example in the test? This can be a template.

https://github.com/MadNLP/MadNLP.jl/blob/master/test/MOI_interface_test.jl#L100-L112

@guimarqu
Copy link
Contributor Author

Thanks for the contribution @guimarqu! Do you mind including your example in the test? This can be a template.

https://github.com/MadNLP/MadNLP.jl/blob/master/test/MOI_interface_test.jl#L100-L112

I added the two examples in the tests. You can remove the code you don't need.

@sshin23 sshin23 merged commit 5df1a02 into MadNLP:master Nov 15, 2024
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.

2 participants