Fixing set_start_value only accept numbers (#2237)

[henriquebecker91]

Intends to close #2237 and some related open issues (#649, #794, #853).

Some guidance is needed. For now, I just edited what I found to be the relevant piece of documentation, and added a new tests as well flags to an old test (allowing more granular selection of which attributes are tested). I would like to know:

1. There are other places where the documentation needs to be changed? JuMP probably will need a change, but there are other points inside MOI?
2. Seems to me that allowing nothing to be passed is something done at the wrapper level, and the generic methods in MOI already allow it. The test will probably force the wrappers to allow this behavior.
3. How is the best way to check if the test is working? Should I download a bunch of wrappers and make they use my version of MOI, and then call their tests?

No hurry, @odow, I do not know if I will work during the week, but I will try finish it next weekend if I am sure of what needs to be done yet.

[odow]

1. We should update the documentation for ConstraintDualStart and ConstraintPrimaStart as well:
   

   MathOptInterface.jl/src/attributes.jl

   Lines 1046 to 1053 in a6274e6

   	A constraint attribute for the initial assignment to some constraint's primal value(s) that the optimizer may use to warm-start the solve.
   	"""
   	struct ConstraintPrimalStart <: AbstractConstraintAttribute end
   	"""
   	ConstraintDualStart()
   	A constraint attribute for the initial assignment to some constraint's dual value(s) that the optimizer may use to warm-start the solve.

2. Rather than modifying the current test, you should add new tests

function test_variable_primal_start(model, config)
    MOI.empty!(model)
    x = MOI.add_variable(model)
    @test MOI.get(model, MOI.VariablePrimalStart(), x) === nothing
    MOI.set(model, MOI.VariablePrimalStart(), x, 1.0)
    @test MOI.get(model, MOI.VariablePrimalStart(), x) == 1.0
    MOI.set(model, MOI.VariablePrimalStart(), x, nothing)
    @test MOI.get(model, MOI.VariablePrimalStart(), x) === nothing
end

3. You can add a test for the test here: https://github.com/jump-dev/MathOptInterface.jl/blob/master/test/Test/modellike.jl
   Something like

model = MOIU.MockOptimizer(MOIU.UniversalFallback(MOIU.Model{Float64}()))
MOI.Test.test_variable_primal_start(model, MOI.Test.TestConfig())

[henriquebecker91]

@odow, you asked to do the same modification of documentation to ConstraintPrimalStart and ConstraintDualStart, ok, I did (not pushed yet), but then you suggest creating just a function test_variable_primal_start(model, config), tests should be added for ConstraintPrimalStart and ConstraintDualStart, no?

[henriquebecker91]

I made a new commit. It updates the documentation of VariablePrimalStart, ConstraintDualStart, and ConstraintPrimalStart to what was suggested by @mlubin.

I also reverted the test I modified and instead created a new test, but not only for VariablePrimalStart (also for the constraint start values), and it is a little more involved than the test suggested by @odow.

I call the new test in the fashion suggested by @odow. The config parameter is not used, should it be used?

My new test method has 5 commented @test calls. They fail if uncommented. I am not sure if this is desired behavior or not. I expect a decision from the maintainers about removing or uncommenting them. In the current situation (with these 5 commented) all tests pass.

[henriquebecker91]

Should I be doing something here?

[henriquebecker91]

Right now, I have a window of opportunity to make any changes necessary for this to be approved, while I wait for some reviews of my last paper. Any directions?

[henriquebecker91]

I followed all @blegat suggestions and did a minor cosmetic change (changed !(... \in ...) to ... \notin ...).

[henriquebecker91]

Screenshot of tests passing:

[blegat]

Can you remove the "Draft" state of the PR ?

[henriquebecker91]

Sorry, forgot about that. Now it is ready for review. I will make that small comment change.