Fix LowerBoundAlreadySet and UpperBoundAlreadySet (#325)

Author: odow

src/MOI_wrapper.jl

@@ -17,11 +17,21 @@ const BOUNDS = Union{
     _kSCIP_SOLVE_STATUS_FINISHED,
 )
 
+@enum(
+    _SCIP_BOUND_TYPE,
+    _kSCIP_EQUAL_TO,
+    _kSCIP_INTERVAL,
+    _kSCIP_LESS_THAN,
+    _kSCIP_GREATER_THAN,
+    _kSCIP_LESS_AND_GREATER_THAN,
+)
+
 mutable struct Optimizer <: MOI.AbstractOptimizer
     inner::SCIPData
     reference::Dict{Ptr{Cvoid},Union{VarRef,ConsRef}}
     constypes::Dict{Tuple{Type,Type},Set{ConsRef}}
-    binbounds::Dict{MOI.VariableIndex,BOUNDS} # only for binary variables
+    binbounds::Dict{MOI.VariableIndex,MOI.Interval{Float64}} # only for binary variables
+    bound_types::Dict{MOI.VariableIndex,_SCIP_BOUND_TYPE}
     params::Dict{String,Any}
     start::Dict{MOI.VariableIndex,Float64} # can be partial
     moi_separator::Any # ::Union{CutCbSeparator, Nothing}
@@ -44,7 +54,8 @@ mutable struct Optimizer <: MOI.AbstractOptimizer
             SCIPData(),
             Dict{Ptr{Cvoid},Union{VarRef,ConsRef}}(),
             Dict{Tuple{Type,Type},Set{ConsRef}}(),
-            Dict{MOI.VariableIndex,BOUNDS}(),
+            Dict{MOI.VariableIndex,MOI.Interval{Float64}}(),
+            Dict{MOI.VariableIndex,_SCIP_BOUND_TYPE}(),
             Dict{String,Any}(),
             Dict{MOI.VariableIndex,Float64}(),
             nothing,
@@ -86,6 +97,7 @@ function MOI.empty!(o::Optimizer)
     empty!(o.reference)
     empty!(o.constypes)
     empty!(o.binbounds)
+    empty!(o.bound_types)
     empty!(o.start)
     o.inner = SCIPData()
     # reapply parameters
@@ -128,13 +140,17 @@ function cons(
 end
 
 "Extract bounds from sets."
-bounds(set::MOI.EqualTo{Float64}) = (set.value, set.value)
+bounds(o, set::MOI.EqualTo{Float64}) = (set.value, set.value)
 
-bounds(set::MOI.GreaterThan{Float64}) = (set.lower, nothing)
+function bounds(o, set::MOI.GreaterThan{Float64})
+    return (set.lower, SCIPinfinity(o))
+end
 
-bounds(set::MOI.LessThan{Float64}) = (nothing, set.upper)
+function bounds(o, set::MOI.LessThan{Float64})
+    return (-SCIPinfinity(o), set.upper)
+end
 
-bounds(set::MOI.Interval{Float64}) = (set.lower, set.upper)
+bounds(o, set::MOI.Interval{Float64}) = (set.lower, set.upper)
 
 "Make set from bounds."
 function from_bounds(::Type{MOI.EqualTo{Float64}}, lower, upper)

src/MOI_wrapper/UserCutCallback.jl

@@ -76,17 +76,16 @@ function MOI.submit(
     func::MOI.ScalarAffineFunction{Float64},
     set::BOUNDS,
 )
-    lhs, rhs = bounds(set)
-    inf = SCIPinfinity(o)
+    lhs, rhs = bounds(o, set)
     add_cut_sepa(
         o.inner.scip[],
         o.inner.vars,
         o.inner.sepas,
         cb_data.callback_data.sepa,
         [VarRef(t.variable.value) for t in func.terms],
         [t.coefficient for t in func.terms],
-        something(lhs, -inf),
-        something(rhs, inf),
+        lhs,
+        rhs,
     )
     cb_data.callback_data.submit_called = true
     return

src/MOI_wrapper/linear_constraints.jl

@@ -22,9 +22,7 @@ function MOI.add_constraint(
     allow_modification(o)
     varrefs = [VarRef(t.variable.value) for t in func.terms]
     coefs = [t.coefficient for t in func.terms]
-    inf = SCIPinfinity(o)
-    lhs, rhs = bounds(set)
-    lhs, rhs = something(lhs, -inf), something(rhs, inf)
+    lhs, rhs = bounds(o, set)
     cr = add_linear_constraint(o.inner, varrefs, coefs, lhs, rhs)
     ci = MOI.ConstraintIndex{F,S}(cr.val)
     register!(o, ci)
@@ -39,9 +37,7 @@ function MOI.set(
     set::S,
 ) where {S<:BOUNDS}
     allow_modification(o)
-    inf = SCIPinfinity(o)
-    lhs, rhs = bounds(set)
-    lhs, rhs = something(lhs, -inf), something(rhs, inf)
+    lhs, rhs = bounds(o, set)
     @SCIP_CALL SCIPchgLhsLinear(o, cons(o, ci), lhs)
     @SCIP_CALL SCIPchgRhsLinear(o, cons(o, ci), rhs)
     return nothing

src/MOI_wrapper/quadratic_constraints.jl

@@ -33,9 +33,7 @@ function MOI.add_constraint(
     factor = 1.0 .- 0.5 * (quadrefs1 .== quadrefs2)
     quadcoefs = factor .* [t.coefficient for t in func.quadratic_terms]
     # range
-    inf = SCIPinfinity(o)
-    lhs, rhs = bounds(set)
-    lhs, rhs = something(lhs, -inf), something(rhs, inf)
+    lhs, rhs = bounds(o, set)
     cr = add_quadratic_constraint(
         o.inner,
         linrefs,
@@ -59,9 +57,7 @@ function MOI.set(
     set::S,
 ) where {S<:BOUNDS}
     allow_modification(o)
-    inf = SCIPinfinity(o)
-    lhs, rhs = bounds(set)
-    lhs, rhs = something(lhs, -inf), something(rhs, inf)
+    lhs, rhs = bounds(o, set)
     @SCIP_CALL SCIPchgLhsQuadratic(o, cons(o, ci), lhs)
     @SCIP_CALL SCIPchgRhsQuadratic(o, cons(o, ci), rhs)
     return nothing

src/MOI_wrapper/variable.jl

@@ -115,8 +115,11 @@ function MOI.delete(o::Optimizer, vi::MOI.VariableIndex)
         throw(MOI.InvalidIndex(vi))
     end
     delete!(o.binbounds, vi)
+    delete!(o.bound_types, vi)
     delete!(o.reference, var(o, vi))
     delete(o.inner, VarRef(vi.value))
+    # FIXME(odow): delete the associated ConstraintIndex
+    delete!(o.bound_types, vi)
     o.name_to_variable = nothing
     return nothing
 end
@@ -176,22 +179,11 @@ function MOI.add_constraint(o::Optimizer, vi::MOI.VariableIndex, ::MOI.ZeroOne)
     v = var(o, vi)
     p_infeas = Ref{SCIP_Bool}()
     @SCIP_CALL SCIPchgVarType(o, v, SCIP_VARTYPE_BINARY, p_infeas)
-    # Need to adjust bounds for SCIP, which fails with an error otherwise.
-    # Check for conflicts with existing bounds first:
     lb, ub = SCIPvarGetLbOriginal(v), SCIPvarGetUbOriginal(v)
-    if lb == -SCIPinfinity(o) && ub == SCIPinfinity(o)
-        @SCIP_CALL SCIPchgVarLb(o, v, 0.0)
-        @SCIP_CALL SCIPchgVarUb(o, v, 1.0)
-    else
-        # Store old bounds for later recovery.
-        o.binbounds[vi] = MOI.Interval(lb, ub)
-        if ub > 1.0
-            @SCIP_CALL SCIPchgVarUb(o, v, 1.0)
-        end
-        if lb < 0.0
-            @SCIP_CALL SCIPchgVarLb(o, v, 0.0)
-        end
-    end
+    # Store old bounds for later recovery.
+    o.binbounds[vi] = MOI.Interval(lb, ub)
+    @SCIP_CALL SCIPchgVarLb(o, v, max(lb, 0.0))
+    @SCIP_CALL SCIPchgVarUb(o, v, min(ub, 1.0))
     ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.ZeroOne}(vi.value)
     return register!(o, ci)
 end
@@ -207,10 +199,9 @@ function MOI.delete(
     p_infeas = Ref{SCIP_Bool}()
     @SCIP_CALL SCIPchgVarType(o, v, SCIP_VARTYPE_CONTINUOUS, p_infeas)
     bounds = get(o.binbounds, vi, nothing)
-    if bounds !== nothing
-        @SCIP_CALL SCIPchgVarLb(o, v, bounds.lower)
-        @SCIP_CALL SCIPchgVarUb(o, v, bounds.upper)
-    end
+    @SCIP_CALL SCIPchgVarLb(o, v, bounds.lower)
+    @SCIP_CALL SCIPchgVarUb(o, v, bounds.upper)
+    delete!(o.binbounds, vi)
     delete!(o.constypes[MOI.VariableIndex, MOI.ZeroOne], ConsRef(ci.value))
     return nothing
 end
@@ -234,38 +225,89 @@ function MOI.supports_constraint(
     return true
 end
 
+function _throw_if_existing_lower(x, type, new_set::S) where {S}
+    if type == _kSCIP_EQUAL_TO
+        throw(MOI.LowerBoundAlreadySet{MOI.EqualTo{Float64},S}(x))
+    elseif type == _kSCIP_GREATER_THAN
+        throw(MOI.LowerBoundAlreadySet{MOI.GreaterThan{Float64},S}(x))
+    elseif type == _kSCIP_LESS_AND_GREATER_THAN
+        throw(MOI.LowerBoundAlreadySet{MOI.GreaterThan{Float64},S}(x))
+    elseif type == _kSCIP_INTERVAL
+        throw(MOI.LowerBoundAlreadySet{MOI.Interval{Float64},S}(x))
+    end
+    return
+end
+
+function _throw_if_existing_upper(x, type, new_set::S) where {S}
+    if type == _kSCIP_EQUAL_TO
+        throw(MOI.UpperBoundAlreadySet{MOI.EqualTo{Float64},S}(x))
+    elseif type == _kSCIP_LESS_THAN
+        throw(MOI.UpperBoundAlreadySet{MOI.LessThan{Float64},S}(x))
+    elseif type == _kSCIP_LESS_AND_GREATER_THAN
+        throw(MOI.UpperBoundAlreadySet{MOI.LessThan{Float64},S}(x))
+    elseif type == _kSCIP_INTERVAL
+        throw(MOI.UpperBoundAlreadySet{MOI.Interval{Float64},S}(x))
+    end
+    return
+end
+
+function _update_bound(o::Optimizer, x, s::MOI.EqualTo, l, u)
+    type = get(o.bound_types, x, nothing)
+    _throw_if_existing_lower(x, type, s)
+    _throw_if_existing_upper(x, type, s)
+    o.bound_types[x] = _kSCIP_EQUAL_TO
+    return s.value, s.value
+end
+
+function _update_bound(o::Optimizer, x, s::MOI.Interval, l, u)
+    type = get(o.bound_types, x, nothing)
+    _throw_if_existing_lower(x, type, s)
+    _throw_if_existing_upper(x, type, s)
+    o.bound_types[x] = _kSCIP_INTERVAL
+    return s.lower, s.upper
+end
+
+function _update_bound(o::Optimizer, x, s::MOI.LessThan, l, u)
+    type = get(o.bound_types, x, nothing)
+    _throw_if_existing_upper(x, type, s)
+    if type == _kSCIP_GREATER_THAN
+        o.bound_types[x] = _kSCIP_LESS_AND_GREATER_THAN
+    else
+        o.bound_types[x] = _kSCIP_LESS_THAN
+    end
+    return l, s.upper
+end
+
+function _update_bound(o::Optimizer, x, s::MOI.GreaterThan, l, u)
+    type = get(o.bound_types, x, nothing)
+    _throw_if_existing_lower(x, type, s)
+    if type == _kSCIP_LESS_THAN
+        o.bound_types[x] = _kSCIP_LESS_AND_GREATER_THAN
+    else
+        o.bound_types[x] = _kSCIP_GREATER_THAN
+    end
+    return s.lower, u
+end
+
 function MOI.add_constraint(
     o::Optimizer,
     vi::MOI.VariableIndex,
     set::S,
 ) where {S<:BOUNDS}
     allow_modification(o)
     v = var(o, vi)
-    newlb, newub = bounds(set)
-    inf = SCIPinfinity(o)
-    newlb, newub = something(newlb, -inf), something(newub, inf)
-    oldlb, oldub = SCIPvarGetLbOriginal(v), SCIPvarGetUbOriginal(v)
-    # FIXME(odow): This section is broken for detecting existing bounds
-    if (oldlb != -inf && newlb != -inf || oldub != inf && newub != inf)
-        if SCIPvarGetType(v) == SCIP_VARTYPE_BINARY
-            # Store new bounds
-            o.binbounds[vi] = MOI.Interval(newlb, newub)
-            if newlb < 0.0
-                newlb = oldlb
-            end
-            if newub > 1.0
-                newub = oldub
-            end
-        else
-            throw(MOI.LowerBoundAlreadySet{S,S}(vi))
-        end
-    end
-    if newlb != -inf
-        @SCIP_CALL SCIPchgVarLb(o, v, newlb)
+    l, u = SCIPvarGetLbOriginal(v), SCIPvarGetUbOriginal(v)
+    if SCIPvarGetType(v) == SCIP_VARTYPE_BINARY
+        s = o.binbounds[vi]
+        l, u = s.lower, s.upper
     end
-    if newub != inf
-        @SCIP_CALL SCIPchgVarUb(o, v, newub)
+    l, u = _update_bound(o, vi, set, l, u)
+    if SCIPvarGetType(v) == SCIP_VARTYPE_BINARY
+        o.binbounds[vi] = MOI.Interval(l, u)
+        l, u = max(0.0, l), min(1.0, u)
     end
+    @SCIP_CALL SCIPchgVarLb(o, v, l)
+    @SCIP_CALL SCIPchgVarUb(o, v, u)
     ci = MOI.ConstraintIndex{MOI.VariableIndex,S}(vi.value)
     return register!(o, ci)
 end
@@ -303,6 +345,14 @@ function MOI.delete(
     end
     # but do delete the constraint reference
     delete!(o.binbounds, vi)
+    type = o.bound_types[vi]
+    if type == _kSCIP_LESS_AND_GREATER_THAN && S <: MOI.LessThan
+        o.bound_types[vi] = _kSCIP_GREATER_THAN
+    elseif type == _kSCIP_LESS_AND_GREATER_THAN && S <: MOI.GreaterThan
+        o.bound_types[vi] = _kSCIP_LESS_AND_GREATER_THAN
+    else
+        delete!(o.bound_types, vi)
+    end
     delete!(o.constypes[MOI.VariableIndex, S], ConsRef(ci.value))
     return nothing
 end
@@ -314,25 +364,25 @@ function MOI.set(
     set::S,
 ) where {S<:BOUNDS}
     allow_modification(o)
-    v = var(o, MOI.VariableIndex(ci.value))
-    lb, ub = bounds(set)
-    old_interval =
-        get(o.binbounds, MOI.VariableIndex(ci.value), MOI.Interval(0.0, 1.0))
-    if lb !== nothing
+    vi = MOI.VariableIndex(ci.value)
+    v = var(o, vi)
+    lb, ub = bounds(o, set)
+    inf = SCIPinfinity(o)
+    if lb > -inf
         if SCIPvarGetType(v) == SCIP_VARTYPE_BINARY
+            old_interval = o.binbounds[vi]
+            o.binbounds[vi] = MOI.Interval(lb, old_interval.upper)
             lb = max(lb, 0.0)
         end
         @SCIP_CALL SCIPchgVarLb(o, v, lb)
-        o.binbounds[MOI.VariableIndex(ci.value)] =
-            MOI.Interval(lb, old_interval.upper)
     end
-    if ub !== nothing
+    if ub < inf
         if SCIPvarGetType(v) == SCIP_VARTYPE_BINARY
+            old_interval = o.binbounds[vi]
+            o.binbounds[vi] = MOI.Interval(old_interval.lower, ub)
             ub = min(ub, 1.0)
         end
         @SCIP_CALL SCIPchgVarUb(o, v, ub)
-        o.binbounds[MOI.VariableIndex(ci.value)] =
-            MOI.Interval(old_interval.lower, ub)
     end
     return nothing
 end