[Bridges] throw for bound already set on bridged variable (#2383)

Author: blegat

src/Bridges/Variable/map.jl

@@ -44,6 +44,8 @@ mutable struct Map <: AbstractDict{MOI.VariableIndex,AbstractBridge}
     # `(ci::ConstraintIndex{MOI.VectorOfVariables}).value` ->
     # the dimension of the set
     vector_of_variables_length::Vector{Int64}
+    # Same as in `MOI.Utilities.VariablesContainer`
+    set_mask::Vector{UInt16}
 end
 
 function Map()
@@ -58,6 +60,7 @@ function Map()
         Dict{MOI.ConstraintIndex,Int64}(),
         Int64[],
         Int64[],
+        UInt16[],
     )
 end
 
@@ -88,6 +91,7 @@ function Base.empty!(map::Map)
     empty!(map.constraint_context)
     empty!(map.vector_of_variables_map)
     empty!(map.vector_of_variables_length)
+    empty!(map.set_mask)
     return map
 end
 
@@ -113,8 +117,9 @@ end
 function Base.delete!(map::Map, vi::MOI.VariableIndex)
     if iszero(map.info[-vi.value])
         # Delete scalar variable
-        map.bridges[bridge_index(map, vi)] = nothing
-        map.sets[bridge_index(map, vi)] = nothing
+        index = bridge_index(map, vi)
+        map.bridges[index] = nothing
+        map.sets[index] = nothing
     elseif has_keys(map, [vi])
         # Delete whole vector
         delete!(map, [vi])
@@ -131,6 +136,7 @@ function Base.delete!(map::Map, vi::MOI.VariableIndex)
             map.index_in_vector[i] -= 1
         end
     end
+    map.set_mask[-vi.value] = MOI.Utilities._DELETED_VARIABLE
     map.index_in_vector[-vi.value] = -1
     return map
 end
@@ -144,6 +150,7 @@ function Base.delete!(map::Map, vis::Vector{MOI.VariableIndex})
         )
     end
     for vi in vis
+        map.set_mask[-vi.value] = MOI.Utilities._DELETED_VARIABLE
         map.index_in_vector[-vi.value] = -1
     end
     map.bridges[bridge_index(map, first(vis))] = nothing
@@ -274,6 +281,64 @@ function MOI.is_valid(
            map.sets[index] === S
 end
 
+"""
+    MOI.add_constraint(map::Map, vi::MOI.VariableIndex, set::MOI.AbstractScalarSet)
+
+Record that a constraint `vi`-in-`set` is added and throws if a lower or upper bound
+is set by this constraint and such bound has already been set for `vi`.
+"""
+function MOI.add_constraint(::Map, ::MOI.VariableIndex, ::MOI.AbstractScalarSet)
+    # Nothing to do as this is is not recognized as setting a lower or upper bound
+end
+
+# We cannot use `SUPPORTED_VARIABLE_SCALAR_SETS` because
+# `Integer` and `ZeroOne` do not define `T` and we need `T`
+# for `_throw_if_lower_bound_set`.
+const _BOUNDED_VARIABLE_SCALAR_SETS{T} = Union{
+    MOI.EqualTo{T},
+    MOI.GreaterThan{T},
+    MOI.LessThan{T},
+    MOI.Interval{T},
+    MOI.Semicontinuous{T},
+    MOI.Semiinteger{T},
+    MOI.Parameter{T},
+}
+
+function MOI.add_constraint(
+    map::Map,
+    vi::MOI.VariableIndex,
+    ::S,
+) where {T,S<:_BOUNDED_VARIABLE_SCALAR_SETS{T}}
+    flag = MOI.Utilities._single_variable_flag(S)
+    index = -vi.value
+    mask = map.set_mask[index]
+    MOI.Utilities._throw_if_lower_bound_set(vi, S, mask, T)
+    MOI.Utilities._throw_if_upper_bound_set(vi, S, mask, T)
+    map.set_mask[index] = mask | flag
+    return
+end
+
+"""
+    delete(map::Map, ci::MOI.ConstraintIndex{MOI.VariableIndex,<:MOI.AbstractScalarSet})
+
+Record that the constraint `vi`-in-`S` is deleted.
+"""
+function MOI.delete(
+    ::Map,
+    ci::MOI.ConstraintIndex{MOI.VariableIndex,<:MOI.AbstractScalarSet},
+)
+    # Nothing to do as this is is not recognized as setting a lower or upper bound
+end
+
+function MOI.delete(
+    map::Map,
+    ci::MOI.ConstraintIndex{MOI.VariableIndex,S},
+) where {T,S<:_BOUNDED_VARIABLE_SCALAR_SETS{T}}
+    flag = MOI.Utilities._single_variable_flag(S)
+    map.set_mask[-ci.value] &= ~flag
+    return
+end
+
 """
     constraints_with_set(map::Map, S::Type{<:MOI.AbstractSet})
 
@@ -384,6 +449,7 @@ function add_key_for_bridge(
     push!(map.index_in_vector, 0)
     push!(map.bridges, nothing)
     push!(map.sets, typeof(set))
+    push!(map.set_mask, 0x0000)
     map.bridges[bridge_index] = call_in_context(map, bridge_index, bridge_fun)
     index = -bridge_index
     variable = MOI.VariableIndex(index)
@@ -400,6 +466,7 @@ function add_key_for_bridge(
             end
         end
     end
+    MOI.add_constraint(map, variable, set)
     return variable, MOI.ConstraintIndex{MOI.VariableIndex,typeof(set)}(index)
 end
 
@@ -443,12 +510,14 @@ function add_keys_for_bridge(
     push!(map.index_in_vector, 1)
     push!(map.bridges, nothing)
     push!(map.sets, typeof(set))
+    push!(map.set_mask, 0x0000)
     for i in 2:MOI.dimension(set)
         push!(map.parent_index, 0)
         push!(map.info, i)
         push!(map.index_in_vector, i)
         push!(map.bridges, nothing)
         push!(map.sets, nothing)
+        push!(map.set_mask, 0x0000)
     end
     map.bridges[bridge_index] = call_in_context(map, bridge_index, bridge_fun)
     variables = MOI.VariableIndex[

src/Bridges/bridge_optimizer.jl

@@ -671,7 +671,10 @@ function MOI.delete(b::AbstractBridgeOptimizer, vi::MOI.VariableIndex)
     return
 end
 
-function MOI.delete(b::AbstractBridgeOptimizer, ci::MOI.ConstraintIndex)
+function MOI.delete(
+    b::AbstractBridgeOptimizer,
+    ci::MOI.ConstraintIndex{F},
+) where {F}
     if is_bridged(b, ci)
         MOI.throw_if_not_valid(b, ci)
         br = bridge(b, ci)
@@ -687,6 +690,11 @@ function MOI.delete(b::AbstractBridgeOptimizer, ci::MOI.ConstraintIndex)
         else
             delete!(Constraint.bridges(b)::Constraint.Map, ci)
         end
+        if F === MOI.VariableIndex && ci.value < 0
+            # Constraint on a bridged variable so we need to remove the flag
+            # if it is a bound
+            MOI.delete(Variable.bridges(b), ci)
+        end
         Variable.call_in_context(
             Variable.bridges(b),
             ci,
@@ -1838,6 +1846,7 @@ function MOI.add_constraint(
                     typeof(f),
                     typeof(s),
                 )
+                MOI.add_constraint(Variable.bridges(b), f, s)
                 return add_bridged_constraint(b, BridgeType, f, s)
             end
         elseif f isa MOI.VectorOfVariables

test/Bridges/Variable/map.jl

@@ -279,6 +279,21 @@ function test_EmptyMap()
     return
 end
 
+function test_double_variable_bound()
+    map = MOI.Bridges.Variable.Map()
+    b1 = VariableDummyBridge(1)
+    set1 = MOI.EqualTo(0.0)
+    v1, c1 = MOI.Bridges.Variable.add_key_for_bridge(map, () -> b1, set1)
+    MOI.is_valid(map, c1)
+    MOI.add_constraint(map, v1, MOI.Integer())
+    cint = MOI.ConstraintIndex{typeof(v1),MOI.Integer}(v1.value)
+    MOI.delete(map, cint)
+    set2 = MOI.LessThan(1.0)
+    err = MOI.UpperBoundAlreadySet{typeof(set1),typeof(set2)}(v1)
+    @test_throws err MOI.add_constraint(map, v1, set2)
+    return
+end
+
 end  # module
 
 TestVariableMap.runtests()

test/Bridges/lazy_bridge_optimizer.jl

@@ -316,14 +316,7 @@ function test_MOI_runtests_StandardSDPAModel()
         model,
         MOI.Test.Config(
             exclude = Any[MOI.optimize!, MOI.SolverName, MOI.SolverVersion],
-        );
-        exclude = String[
-            # Skip these tests because the bridge reformulates bound
-            # constraints, so there is no conflict. An error _is_ thrown if two
-            # sets of the same type are added.
-            "test_model_LowerBoundAlreadySet",
-            "test_model_UpperBoundAlreadySet",
-        ],
+        ),
     )
     return
 end