Skip to content

Conversation

frapac
Copy link
Contributor

@frapac frapac commented Sep 9, 2021

Minor fixes to make the nonlinear tests pass both with Knitro and MadNLP.

  1. NLPBlockDual: Check correctness of dual variables directly with KKT stationary condition

As the problem HS071 is nonconvex, solvers can return a different values than the one previously specified in test_nonlinear_hs071_NLPBlockDual:

@test isapprox(dual, [0.178761800, 0.985000823], config)

For instance, both Knitro and MadNLP return [0.0, 0.4058829] there. Using directly the KKT stationary condition avoids this problem.

  1. Fix issue in definition of FeasibilitySenseEvaluator
    When using the FeasibilitySenseEvaluator, we test that the primal variable returned is equal to 1:
    xv = MOI.get(model, MOI.VariablePrimal(), x)
    @test isapprox(abs(xv), 1.0, config)

However, this is inconsistent with the definition of the upper and lower bounds:

    lb = [1.0]
    ub = [2.0]

Indeed, the constraint specified inside the FeasibilitySenseEvaluator is then 1 <= x^2 <= 2, and x can be any value between 1.0 and sqrt(2) (or -sqrt(2) and -1.0). To avoid this, we change the upper bound to 1.0 so the constraint now specified is x^2 == 1.0, consistent with the FeasibilitySenseEvaluator's docstring:

Test for FEASIBILITY_SENSE.
Find x satisfying x^2 == 1.

- NLPBlockDual: Check correctness of dual variables with KKT stationarity condition
- Fix issue in definition of FeasibilityEvaluator
- Nonlinear tests now passes with Knitro
@odow odow added the Submodule: Tests About the Tests submodule label Sep 9, 2021
@frapac
Copy link
Contributor Author

frapac commented Sep 14, 2021

I just fixed an issue with the sign when the sense is set to MAX_SENSE (for some reason, Knitro was running only MIN_SENSE, then throwed an error and exited silently).

I am not sure to understand the issue with the MockOptimizer. When running the MOI tests, the test test_nonlinear_hs071_NLPBlockDual is currently breaking, with the following error:

 Got exception outside of a @test
  Fallback getter for variable constraint dual does not support other variable-wise constraints on the variable. Please report this issue to the solver wrapper package.
  Stacktrace:
    [1] error(::String, ::String, ::String)
      @ Base ./error.jl:42
    [2] variable_dual(model::MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, #unused#::MathOptInterface.ConstraintDual, ci::MathOptInterface.ConstraintIndex
{MathOptInterface.VariableIndex, MathOptInterface.GreaterThan{Float64}}, vi::MathOptInterface.VariableIndex, F::Type{MathOptInterface.VariableIndex}, S::Type{MathOptInterface.LessThan{Float64}})
      @ MathOptInterface.Utilities ~/.julia/dev/MathOptInterface/src/Utilities/results.jl:385
    [3] variable_dual(model::MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, attr::MathOptInterface.ConstraintDual, ci::MathOptInterface.ConstraintIndex{Mat
hOptInterface.VariableIndex, MathOptInterface.GreaterThan{Float64}}, vi::MathOptInterface.VariableIndex)
      @ MathOptInterface.Utilities ~/.julia/dev/MathOptInterface/src/Utilities/results.jl:446
    [4] get_fallback(model::MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, attr::MathOptInterface.ConstraintDual, ci::MathOptInterface.ConstraintIndex{Math
OptInterface.VariableIndex, MathOptInterface.GreaterThan{Float64}})
      @ MathOptInterface.Utilities ~/.julia/dev/MathOptInterface/src/Utilities/results.jl:482
    [5] get
      @ ~/.julia/dev/MathOptInterface/src/Utilities/mockoptimizer.jl:663 [inlined]
    [6] _broadcast_getindex_evalf
      @ ./broadcast.jl:648 [inlined]
    [7] _broadcast_getindex
      @ ./broadcast.jl:621 [inlined]
    [8] getindex
      @ ./broadcast.jl:575 [inlined]
    [9] copy
      @ ./broadcast.jl:922 [inlined]
   [10] materialize
      @ ./broadcast.jl:883 [inlined]
   [11] get(model::MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, attr::MathOptInterface.ConstraintDual, idxs::Vector{MathOptInterface.ConstraintIndex{Math
OptInterface.VariableIndex, MathOptInterface.GreaterThan{Float64}}})
      @ MathOptInterface ~/.julia/dev/MathOptInterface/src/attributes.jl:305
   [12] test_nonlinear_hs071_NLPBlockDual(model::MathOptInterface.Utilities.MockOptimizer{MathOptInterface.Utilities.UniversalFallback{MathOptInterface.Utilities.Model{Float64}}}, config::MathOptInterface.Test.Config{Float64})
      @ MathOptInterface.Test ~/.julia/dev/MathOptInterface/src/Test/test_nonlinear.jl:473
   [13] macro expansion

The problem is a bit obscure to me. If I comment out these lines, then I don't get an error (but test is breaking because by default MockOptimizer returns 0.0 for the dual of the bound constraints). Do you have any idea on how to fix this issue?

@odow
Copy link
Member

odow commented Sep 14, 2021

Try adding this

model.eval_variable_constraint_dual = false
return () -> model.eval_variable_constraint_dual = true

@frapac
Copy link
Contributor Author

frapac commented Sep 14, 2021

Indeed, it looks better now. The test has been modified to include the dual solutions for the variable constraints as well:

MOI.Utilities.mock_optimize!(
    mock,
    config.optimal_status,
    [
        1.1,
        1.5735520521364397,
        2.6746837915674373,
        5.4,
    ],
    (MOI.VariableIndex, MOI.GreaterThan{Float64}) =>
        [28.590703804861093, 0.0, 0.0, 0.0],
    (MOI.VariableIndex, MOI.LessThan{Float64}) =>
        [0.0, 0.0, 0.0, -5.582550550234756],
)
MOI.set(mock, MOI.NLPBlockDual(), [0.178761800, 0.985000823])

(the primal and dual values are computed with Ipopt).

Without the two lines you pointed me before:

 model.eval_variable_constraint_dual = false 
 return () -> model.eval_variable_constraint_dual = true 

the test is breaking again. But I am not sure to understand what these two lines are doing exactly.

@odow
Copy link
Member

odow commented Sep 14, 2021

See the formatting failure. The two lines are just some magic that we need for the mock optimizer. It's a bit of a pain to remember to add it, but it's not user-facing.

@odow odow merged commit 786db51 into jump-dev:master Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Submodule: Tests About the Tests submodule
Development

Successfully merging this pull request may close these issues.

2 participants