Add a unit test for double optimize call

[blegat]

We should add a unit test that calls MOI.optimize! twice. This would have caught this ECOS bug: jump-dev/ECOS.jl#72

[shadiakiki1986]

I tried looking into resolving this issue as a good first issue, but I'm confused about it. Note that I'm new to Julia.

It seems to me that the double-optimize bug was ECOS-specific because the optimize! function is defined in the optimizer. For example:

• the GLPK definition (here) will just re-run the optimize code on every call.
• the ECOS optimizer's definition (here) will check for a data field being nothing and skip the re-run.
• The MOI.CachingOptimizer implementation (here) seems to just re-run the optimization

OTOH, the optimize! definition for AbstractOptimizer (here) is empty. I would have expected some "base code" here about checking if the optimizer has run or not, and then maybe a unit test for that functionality. But the fact that it's empty is leaving me confused.

Could you shed some light for me? Forgive me if this is more work for you to explain to me than to actually resolve the issue. Feel free to just ignore me in that case.

[odow]

Hi there!

All solvers use the tests in this repository as extra correctness tests for themselves. If we find a bug in one solver, instead of adding a test to that particular repository, we add it here so that all solvers can benefit.

For this test, we care about correctness, rather than performance. We don't expect solvers to efficiently decide that they have already solved the problem, only that calling optimize! twice doesn't throw an error or give the wrong answer.

To give an example, ECOS errored calling optimize!(model); optimize!(model). We could add a test to ECOS.jl, but that would only stop us from re-introducing the bug to ECOS.jl in the future. Instead if we add a test here, then all solvers (e.g., SCS.jl, Gurobi.jl, Mosek.jl, ...) will also check that they handle a double optimize call!

To resolve this issue, there are two steps:

1. Add a new test to this file:
   https://github.com/jump-dev/MathOptInterface.jl/blob/master/src/Test/UnitTests/solve.jl
   The test should be something like

function solve_twice(model::MOI.ModelLike, config::TestConfig)
    MOI.empty!(model)
    x = MOI.add_variable(model)
    c = MOI.add_constraint(model, MOI.SingleVariable(x), MOI.GreaterThan(1.0))
    MOI.set(model, MOI.ObjectiveSense(), MOI.MIN_SENSE)
    MOI.set(model, MOI.ObjectiveFunction{MOI.SingleVariable}(), MOI.SingleVariable(x))
    if config.solve
        MOI.optimize!(model)
        MOI.optimize!(model)
        MOI.get(model, MOI.TerminationStatus()) == MOI.OPTIMAL
        MOI.get(model, MOI.VariablePrimal(), x) == 1.0
    end
end
unittests["solve_twice"] = solve_twice

2. Add a test for the test you just wrote (We test the tests!)

• Add the name of the test ("solve_twice") to the end of this array
  

  MathOptInterface.jl/test/Test/unit.jl

  Lines 51 to 52 in 7543afe

  	"solve_result_index",
  	])

• Add a test for the test after this line:
  

  MathOptInterface.jl/test/Test/unit.jl

  Line 394 in 7543afe

  end

  
  The test should look something like

    @testset "solve_twice" begin
        MOIU.set_mock_optimize!(mock,
            (mock::MOIU.MockOptimizer) -> MOIU.mock_optimize!(
                mock,
                MOI.OPTIMAL,
                (MOI.FEASIBLE_POINT, [1.0]),
            ),
            (mock::MOIU.MockOptimizer) -> MOIU.mock_optimize!(
                mock,
                MOI.OPTIMAL,
                (MOI.FEASIBLE_POINT, [1.0]),
            )
        )
        MOIT.solve_twice(mock, config)
    end

Please let me know if you need more help.

[shadiakiki1986]

Thanks for the clear instructions. I just submitted PR 1186 with it.

Other than the notes that I added in the PR:

1. I tried re-running the tests for "SCS.jl, Gurobi.jl, Mosek.jl, ..." to check if any of them would fail with the new unit test of double optimize call.
   1a. Do you have a full list of them somewhere?
   1b. Does anything automatically re-test all these packages upon adding a new unit test to MOI? I think some problems will only show up upon new commits to the individual packages once they "upgrade" their MOI dependency.
2. I couldn't run the gurobi tests because I didn't have gurobi installed
3. Mosek had too much details showing up when I ran the tests, even though I didn't specify any verbosity. This kind of cluttered my view of what tests ran. This point is just an observation.

[shadiakiki1986]

@odow This can be closed now since you merged PR #1186

[odow]

Thanks! You can have this happen automatically in future by including Closes #1186 in the description of the pull request.

Closed by #1186.