Enforce [0,1] bounds on binary variables for LP relaxation

[bfpc]

We also take care if (for whatever reason) the user has already provided
lower/upper bounds.
This reduces a bit of type information on relax_integrality and
enfroce_integrality.

Thanks to @iagoleal

[codecov]

Codecov Report

Merging #236 into master will increase coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
+ Coverage   93.62%   93.73%   +0.11%     
==========================================
  Files          14       14              
  Lines        1207     1229      +22     
==========================================
+ Hits         1130     1152      +22     
  Misses         77       77

Impacted Files	Coverage Δ
src/algorithm.jl	97.82% <100%> (+0.13%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e88935...782aba2. Read the comment docs.

[odow]

Good catch. I found this the other day, but I forgot to fix it or open an issue.

[odow]

Instead of this example, let's add a more targeted test to test/user_interface.jl (I haven't run, so there might be bugs)

@testset "relax_integrality and enforce_integrality" begin
    model = SDDP.LinearPolicyGraph(
        stages = 2, lower_bound = 0.0, 
       optimizer = JuMP.with_optimizer(GLPK.Optimizer)
    ) do sp, t
	    @variable(sp, x, SDDP.State, initial_value = 2.0)
        @variable(sp, b1, Bin)
        @variable(sp, 0.2 <= b2, Bin)
        @variable(sp, 0.5 <= b3 <= 1.2, Bin)
	    @stageobjective(sp, b1 + b2 + b2)
    end
    
    for node in [model[1], model[2]]
        @test JuMP.is_binary(node.subproblem[:b1])
        @test !JuMP.has_lower_bound(model.subproblem[:b1])
        @test !JuMP.has_upper_bound(model.subproblem[:b1])

        @test JuMP.is_binary(node.subproblem[:b2])
        @test JuMP.lower_bound(node.subproblem[:b2]) == 0.2
        @test !JuMP.has_upper_bound(model.subproblem[:b2])

        @test JuMP.is_binary(node.subproblem[:b3])
        @test JuMP.lower_bound(node.subproblem[:b3]) == 0.5
        @test JuMP.upper_bound(node.subproblem[:b3]) == 1.2
    end
    binaries, integers = SDDP.relax_integrality(model)
    for node in [model[1], model[2]]
        @test !JuMP.is_binary(node.subproblem[:b1])
        @test JuMP.lower_bound(model.subproblem[:b1]) == 0.0
        @test JuMP.upper_bound(model.subproblem[:b1]) == 1.0

        @test JuMP.is_binary(node.subproblem[:b2])
        @test JuMP.lower_bound(node.subproblem[:b2]) == 0.2
        @test JuMP.upper_bound(model.subproblem[:b2]) == 1.0

        @test JuMP.is_binary(node.subproblem[:b3])
        @test JuMP.lower_bound(node.subproblem[:b3]) == 0.5
        @test JuMP.upper_bound(node.subproblem[:b3]) == 1.0
    end
    SDDP.enforce_integrality(model, binaries, integers)
    for node in [model[1], model[2]]
        @test JuMP.is_binary(node.subproblem[:b1])
        @test !JuMP.has_lower_bound(model.subproblem[:b1])
        @test !JuMP.has_upper_bound(model.subproblem[:b1])

        @test JuMP.is_binary(node.subproblem[:b2])
        @test JuMP.lower_bound(node.subproblem[:b2]) == 0.2
        @test !JuMP.has_upper_bound(model.subproblem[:b2])

        @test JuMP.is_binary(node.subproblem[:b3])
        @test JuMP.lower_bound(node.subproblem[:b3]) == 0.5
        @test JuMP.upper_bound(node.subproblem[:b3]) == 1.2
    end
end

[bfpc]

I know the rewrite is mostly for binary variables, but should we also exercise the integer part on the test?

[bfpc]

Also, should that be on user_interface.jl or on algorithm.jl (since relax/enforce belong to the latter)?

[odow]

Ah, you got me. Both good points:

• yes, add some tests for integer variables
• yes, in test/algorithm.jl

[bfpc]

Sooooo

GLPK.jl is bugging me. For some reason, it simply ignores the lower bound in @variable(m, 0.2 <= b2, Bin), and the test even before calling relax_integrality() fails. For a non-direct model, as well as for Gurobi models, the bounds are indeed taken into account.

[odow]

The tests passed on Travis? Or are you talking about something else? What do you mean "ignores"? When you query it you get a different value? Or the optimal solution doesn't respect it?

[bfpc]

Yes, the tests passed (as you can see). The only thing is that I had to add the direct_model = false to the constructor, otherwise they would have (completely) failed.
Re: ignore: during model construction. The tests even before relaxing the integrality constraints fail... Maybe we should open a bug on GLPK.jl, but I have a (much) more pressing one already ;)

[odow]

There are probably bugs like that. I have a re-write of the GLPK wrapper which should tidy up lots of mistakes: jump-dev/GLPK.jl#101. It should be released soon-ish.

[odow]

Bump. Do you have time to finish this up? Otherwise I can do it.

[odow]

Thanks for tidying this up. Just waiting on your GLPK query (and green on Travis) and then I'm happy to merge this.

[odow]

Thanks @bfpc!