[Bridges] add final_touch

docs/src/submodules/Bridges/reference.md

@@ -27,6 +27,7 @@ get(::Bridges.AbstractBridge, ::NumberOfVariables)
 get(::Bridges.AbstractBridge, ::ListOfVariableIndices)
 get(::Bridges.AbstractBridge, ::NumberOfConstraints)
 get(::Bridges.AbstractBridge, ::ListOfConstraintIndices)
+Bridges.needs_final_touch
 ```
 
 ## Constraint bridge API

src/Bridges/Constraint/Constraint.jl

@@ -8,7 +8,7 @@ module Constraint
 
 import LinearAlgebra
 import MathOptInterface
-import OrderedCollections: OrderedDict
+import OrderedCollections: OrderedDict, OrderedSet
 import SparseArrays
 
 const MOI = MathOptInterface

src/Bridges/Constraint/bridges/semi_to_binary.jl

@@ -75,6 +75,19 @@ function bridge_constraint(
     f::MOI.VariableIndex,
     s::S,
 ) where {T<:Real,S<:Union{MOI.Semicontinuous{T},MOI.Semiinteger{T}}}
+    F = MOI.VariableIndex
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.GreaterThan{T}}(f.value))
+        throw(MOI.LowerBoundAlreadySet{MOI.GreaterThan{T},S}(f))
+    end
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.LessThan{T}}(f.value))
+        throw(MOI.UpperBoundAlreadySet{MOI.LessThan{T},S}(f))
+    end
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.EqualTo{T}}(f.value))
+        throw(MOI.LowerBoundAlreadySet{MOI.EqualTo{T},S}(f))
+    end
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.Interval{T}}(f.value))
+        throw(MOI.LowerBoundAlreadySet{MOI.Interval{T},S}(f))
+    end
     binary, binary_con = MOI.add_constrained_variable(model, MOI.ZeroOne())
     # var - LB * bin >= 0
     lb = MOI.Utilities.operate(*, T, -s.lower, binary)
@@ -327,3 +340,25 @@ function MOI.get(
 ) where {T,S}
     return [b.lower_bound_index]
 end
+
+MOI.Bridges.needs_final_touch(b::SemiToBinaryBridge) = true
+
+function MOI.Utilities.final_touch(
+    b::SemiToBinaryBridge{T,S},
+    model::MOI.ModelLike,
+) where {T,S}
+    F, f = MOI.VariableIndex, b.variable
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.GreaterThan{T}}(f.value))
+        throw(MOI.LowerBoundAlreadySet{S,MOI.GreaterThan{T}}(f))
+    end
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.LessThan{T}}(f.value))
+        throw(MOI.UpperBoundAlreadySet{S,MOI.LessThan{T}}(f))
+    end
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.EqualTo{T}}(f.value))
+        throw(MOI.LowerBoundAlreadySet{S,MOI.EqualTo{T}}(f))
+    end
+    if MOI.is_valid(model, MOI.ConstraintIndex{F,MOI.Interval{T}}(f.value))
+        throw(MOI.LowerBoundAlreadySet{S,MOI.Interval{T}}(f))
+    end
+    return
+end

src/Bridges/Constraint/map.jl

@@ -20,12 +20,14 @@ struct Map <: AbstractDict{MOI.ConstraintIndex,AbstractBridge}
     # of creation so we need `OrderedDict` and not `Dict`.
     # For `VariableIndex` constraints: (variable, set type) -> bridge
     single_variable_constraints::OrderedDict{Tuple{Int64,Type},AbstractBridge}
+    needs_final_touch::OrderedDict{Type,OrderedSet}
 
     function Map()
         return new(
             Union{Nothing,AbstractBridge}[],
             Tuple{Type,Type}[],
             OrderedDict{Tuple{Int64,Type},AbstractBridge}(),
+            OrderedDict{Type,OrderedSet}(),
         )
     end
 end
@@ -48,6 +50,7 @@ function Base.empty!(map::Map)
     empty!(map.bridges)
     empty!(map.constraint_types)
     empty!(map.single_variable_constraints)
+    empty!(map.needs_final_touch)
     return map
 end
 
@@ -83,6 +86,7 @@ function Base.getindex(
 end
 
 function Base.delete!(map::Map, ci::MOI.ConstraintIndex)
+    _unregister_for_final_touch(map, map.bridges[_index(ci)])
     map.bridges[_index(ci)] = nothing
     return map
 end
@@ -91,6 +95,8 @@ function Base.delete!(
     map::Map,
     ci::MOI.ConstraintIndex{MOI.VariableIndex,S},
 ) where {S}
+    bridge = map.single_variable_constraints[(ci.value, S)]
+    _unregister_for_final_touch(map, bridge)
     delete!(map.single_variable_constraints, (ci.value, S))
     return map
 end
@@ -278,6 +284,7 @@ function add_key_for_bridge(
     ::F,
     ::S,
 ) where {F<:MOI.AbstractFunction,S<:MOI.AbstractSet}
+    _register_for_final_touch(map, bridge)
     push!(map.bridges, bridge)
     push!(map.constraint_types, (F, S))
     return _index(length(map.bridges), F, S)
@@ -289,10 +296,43 @@ function add_key_for_bridge(
     func::MOI.VariableIndex,
     ::S,
 ) where {S<:MOI.AbstractScalarSet}
+    _register_for_final_touch(map, bridge)
     map.single_variable_constraints[(func.value, S)] = bridge
     return MOI.ConstraintIndex{MOI.VariableIndex,S}(func.value)
 end
 
+function _register_for_final_touch(map::Map, bridge::BT) where {BT}
+    if MOI.Bridges.needs_final_touch(bridge)
+        if !haskey(map.needs_final_touch, BT)
+            map.needs_final_touch[BT] = OrderedSet{BT}()
+        end
+        push!(map.needs_final_touch[BT], bridge)
+    end
+    return
+end
+
+function _unregister_for_final_touch(b::Map, bridge::BT) where {BT}
+    if MOI.Bridges.needs_final_touch(bridge)
+        delete!(b.needs_final_touch[BT], bridge)
+    end
+    return
+end
+
+# Function barrier to iterate over bridges of the same type in an efficient way.
+function _final_touch(bridges, model)
+    for bridge in bridges
+        MOI.Utilities.final_touch(bridge, model)
+    end
+    return
+end
+
+function MOI.Utilities.final_touch(map::Map, model::MOI.ModelLike)
+    for bridges in values(map.needs_final_touch)
+        _final_touch(bridges, model)
+    end
+    return
+end
+
 """
     EmptyMap <: AbstractDict{MOI.ConstraintIndex, AbstractBridge}
 
@@ -315,3 +355,5 @@ Base.values(::EmptyMap) = MOI.Utilities.EmptyVector{AbstractBridge}()
 has_bridges(::EmptyMap) = false
 
 number_of_type(::EmptyMap, ::Type{<:MOI.ConstraintIndex}) = 0
+
+MOI.Utilities.final_touch(::MOI.ModelLike, ::EmptyMap) = nothing

src/Bridges/bridge.jl

@@ -253,3 +253,15 @@ MathOptInterface.ScalarAffineFunction{Float64}
 ```
 """
 function set_objective_function_type end
+
+"""
+    needs_final_touch(bridge::AbstractBridge)::Bool
+
+Return whether [`MOI.Utilities.final_touch`](@ref) is implemented by `bridge`.
+
+For example, if the correctness of `bridge` depends on the bounds of a variable
+or the fact that variables are integer, then the bridge can implement
+[`MOI.Utilities.final_touch`](@ref) to check assumptions immediately before a
+call to [`MOI.optimize!`](@ref).
+"""
+needs_final_touch(::AbstractBridge) = false

src/Bridges/bridge_optimizer.jl

@@ -374,7 +374,11 @@ end
 # Implementation of the MOI interface for AbstractBridgeOptimizer
 
 # By convention, the model should be stored in a `model` field
-MOI.optimize!(b::AbstractBridgeOptimizer) = MOI.optimize!(b.model)
+function MOI.optimize!(b::AbstractBridgeOptimizer)
+    MOI.Utilities.final_touch(b)
+    MOI.optimize!(b.model)
+    return
+end
 
 function MOI.is_empty(b::AbstractBridgeOptimizer)
     return isempty(Variable.bridges(b)) &&
@@ -453,8 +457,16 @@ end
 function MOI.supports_incremental_interface(b::AbstractBridgeOptimizer)
     return MOI.supports_incremental_interface(b.model)
 end
-function MOI.Utilities.final_touch(uf::AbstractBridgeOptimizer, index_map)
-    return MOI.Utilities.final_touch(uf.model, index_map)
+
+function MOI.Utilities.final_touch(b::AbstractBridgeOptimizer)
+    MOI.Utilities.final_touch(Constraint.bridges(b), b)
+    return
+end
+
+function MOI.Utilities.final_touch(b::AbstractBridgeOptimizer, index_map)
+    MOI.Utilities.final_touch(b)
+    MOI.Utilities.final_touch(b.model, index_map)
+    return
 end
 
 # References

test/Bridges/Constraint/semi_to_binary.jl

@@ -242,25 +242,65 @@ function test_SemiToBinary()
 end
 
 """
-    test_lower_bound_already_set()
+Test an error is thrown if a bound is added _after_ the semi-xxx constraint.
 
-The second call to `add_constraint` is broken because it should throw:
-```julia
-MOI.LowerBoundAlreadySet{
-    MOI.Semicontinuous{Float64},
-    MOI.GreaterThan{Float64},
-}
-```
-See MathOptInterface issue #1431.
+Note that this means the error is thrown from `Utilties.final_touch`.
 """
-function test_lower_bound_already_set()
-    model = MOI.Utilities.Model{Float64}()
-    bridged = MOI.Bridges.Constraint.SemiToBinary{Float64}(model)
-    x = MOI.add_variable(bridged)
-    MOI.add_constraint(bridged, x, MOI.Semicontinuous(1.0, 2.0))
-    @test_broken(
-        MOI.add_constraint(bridged, x, MOI.GreaterThan(0.0)) === nothing,
-    )
+function test_bounds_already_set_final_touch()
+    for set in [MOI.GreaterThan(0.0), MOI.EqualTo(1.4), MOI.Interval(0.0, 2.0)]
+        model = MOI.Utilities.Model{Float64}()
+        bridged = MOI.Bridges.Constraint.SemiToBinary{Float64}(model)
+        x = MOI.add_variable(bridged)
+        semi_set = MOI.Semicontinuous(1.0, 2.0)
+        MOI.add_constraint(bridged, x, semi_set)
+        MOI.add_constraint(bridged, x, set)
+        @test_throws(
+            MOI.LowerBoundAlreadySet{typeof(semi_set),typeof(set)}(x),
+            MOI.Utilities.final_touch(bridged),
+        )
+    end
+    for set in [MOI.LessThan(2.0)]
+        model = MOI.Utilities.Model{Float64}()
+        bridged = MOI.Bridges.Constraint.SemiToBinary{Float64}(model)
+        x = MOI.add_variable(bridged)
+        semi_set = MOI.Semicontinuous(1.0, 2.0)
+        MOI.add_constraint(bridged, x, semi_set)
+        MOI.add_constraint(bridged, x, set)
+        @test_throws(
+            MOI.UpperBoundAlreadySet{typeof(semi_set),typeof(set)}(x),
+            MOI.Utilities.final_touch(bridged),
+        )
+    end
+    return
+end
+
+"""
+Test an error is thrown if a bound exists prior to adding the semi-xxx
+constraint.
+"""
+function test_bounds_already_set()
+    for set in [MOI.GreaterThan(0.0), MOI.EqualTo(1.4), MOI.Interval(0.0, 2.0)]
+        model = MOI.Utilities.Model{Float64}()
+        bridged = MOI.Bridges.Constraint.SemiToBinary{Float64}(model)
+        x = MOI.add_variable(bridged)
+        semi_set = MOI.Semicontinuous(1.0, 2.0)
+        MOI.add_constraint(bridged, x, set)
+        @test_throws(
+            MOI.LowerBoundAlreadySet{typeof(set),typeof(semi_set)}(x),
+            MOI.add_constraint(bridged, x, semi_set),
+        )
+    end
+    for set in [MOI.LessThan(2.0)]
+        model = MOI.Utilities.Model{Float64}()
+        bridged = MOI.Bridges.Constraint.SemiToBinary{Float64}(model)
+        x = MOI.add_variable(bridged)
+        semi_set = MOI.Semicontinuous(1.0, 2.0)
+        MOI.add_constraint(bridged, x, set)
+        @test_throws(
+            MOI.UpperBoundAlreadySet{typeof(set),typeof(semi_set)}(x),
+            MOI.add_constraint(bridged, x, semi_set)
+        )
+    end
     return
 end