Fix and test that Bridges.final_touch can be called multiple times (#…

…2089)
jump-dev · Feb 6, 2023 · fbf833c · fbf833c
1 parent 79081f0
commit fbf833c
Show file tree

Hide file tree

Showing 11 changed files with 192 additions and 21 deletions.
diff --git a/src/Bridges/Bridges.jl b/src/Bridges/Bridges.jl
@@ -255,6 +255,8 @@ function runtests(
     model = _bridged_model(Bridge, inner)
     MOI.Utilities.loadfromstring!(model, input)
     final_touch(model)
+    # Should be able to call final_touch multiple times.
+    final_touch(model)
     # Load a non-bridged input model, and check that getters are the same.
     test = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}())
     MOI.Utilities.loadfromstring!(test, input)

diff --git a/src/Bridges/Constraint/bridges/bin_packing.jl b/src/Bridges/Constraint/bridges/bin_packing.jl
@@ -59,6 +59,7 @@ struct BinPackingToMILPBridge{
     equal_to::Vector{
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}},
     }
+    bounds::Vector{NTuple{2,T}}
 end
 
 const BinPackingToMILP{T,OT<:MOI.ModelLike} =
@@ -76,6 +77,7 @@ function bridge_constraint(
         MOI.VariableIndex[],
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.LessThan{T}}[],
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}}[],
+        NTuple{2,T}[],
     )
 end
 
@@ -133,6 +135,7 @@ function MOI.delete(model::MOI.ModelLike, bridge::BinPackingToMILPBridge)
     empty!(bridge.equal_to)
     MOI.delete.(model, bridge.variables)
     empty!(bridge.variables)
+    empty!(bridge.bounds)
     return
 end
 
@@ -232,9 +235,7 @@ function MOI.Bridges.final_touch(
     bridge::BinPackingToMILPBridge{T,F},
     model::MOI.ModelLike,
 ) where {T,F}
-    # Clear any existing reformulations!
-    MOI.delete(model, bridge)
-    S = Dict{T,Vector{Tuple{Float64,MOI.VariableIndex}}}()
+    S = Dict{T,Vector{Tuple{T,MOI.VariableIndex}}}()
     scalars = collect(MOI.Utilities.eachscalar(bridge.f))
     bounds = Dict{MOI.VariableIndex,NTuple{2,T}}()
     for i in 1:length(scalars)
@@ -246,41 +247,56 @@ function MOI.Bridges.final_touch(
                 "function has a non-finite domain: $x",
             )
         end
+        if length(bridge.bounds) < i
+            # This is the first time calling final_touch
+            push!(bridge.bounds, ret)
+        elseif bridge.bounds[i] == ret
+            # We've called final_touch before, and the bounds match. No need to
+            # reformulate a second time.
+            continue
+        elseif bridge.bounds[i] != ret
+            # There is a stored bound, and the current bounds do not match. This
+            # means the model has been modified since the previous call to
+            # final_touch. We need to delete the bridge and start again.
+            MOI.delete(model, bridge)
+            MOI.Bridges.final_touch(bridge, model)
+            return
+        end
         unit_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         convex_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         for xi in ret[1]::T:ret[2]::T
             new_var, _ = MOI.add_constrained_variable(model, MOI.ZeroOne())
             push!(bridge.variables, new_var)
             if !haskey(S, xi)
-                S[xi] = Tuple{Float64,MOI.VariableIndex}[]
+                S[xi] = Tuple{T,MOI.VariableIndex}[]
             end
             push!(S[xi], (bridge.s.weights[i], new_var))
-            push!(unit_f.terms, MOI.ScalarAffineTerm(T(-xi), new_var))
-            push!(convex_f.terms, MOI.ScalarAffineTerm(one(T), new_var))
+            push!(unit_f.terms, MOI.ScalarAffineTerm{T}(T(-xi), new_var))
+            push!(convex_f.terms, MOI.ScalarAffineTerm{T}(one(T), new_var))
         end
         push!(
             bridge.equal_to,
             MOI.Utilities.normalize_and_add_constraint(
                 model,
                 MOI.Utilities.operate(+, T, x, unit_f),
-                MOI.EqualTo(zero(T));
+                MOI.EqualTo{T}(zero(T));
                 allow_modify_function = true,
             ),
         )
         push!(
             bridge.equal_to,
-            MOI.add_constraint(model, convex_f, MOI.EqualTo(one(T))),
+            MOI.add_constraint(model, convex_f, MOI.EqualTo{T}(one(T))),
         )
     end
     # We use a sort so that the model order is deterministic.
     for s in sort!(collect(keys(S)))
         ci = MOI.add_constraint(
             model,
-            MOI.ScalarAffineFunction(
-                [MOI.ScalarAffineTerm(w, z) for (w, z) in S[s]],
+            MOI.ScalarAffineFunction{T}(
+                [MOI.ScalarAffineTerm{T}(w, z) for (w, z) in S[s]],
                 zero(T),
             ),
-            MOI.LessThan(bridge.s.capacity),
+            MOI.LessThan{T}(bridge.s.capacity),
         )
         push!(bridge.less_than, ci)
     end

diff --git a/src/Bridges/Constraint/bridges/count_belongs.jl b/src/Bridges/Constraint/bridges/count_belongs.jl
@@ -59,6 +59,7 @@ mutable struct CountBelongsToMILPBridge{
     equal_to::Vector{
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}},
     }
+    bounds::Vector{NTuple{2,T}}
     function CountBelongsToMILPBridge{T}(
         f::Union{MOI.VectorOfVariables,MOI.VectorAffineFunction{T}},
         s::MOI.CountBelongs,
@@ -68,6 +69,7 @@ mutable struct CountBelongsToMILPBridge{
             s,
             MOI.VariableIndex[],
             MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}}[],
+            NTuple{2,T}[],
         )
     end
 end
@@ -139,6 +141,7 @@ function MOI.delete(model::MOI.ModelLike, bridge::CountBelongsToMILPBridge)
         MOI.delete(model, x)
     end
     empty!(bridge.variables)
+    empty!(bridge.bounds)
     return
 end
 
@@ -248,6 +251,21 @@ function _unit_expansion(
                 "non-finite domain: $(f[i])",
             )
         end
+        if length(bridge.bounds) < i
+            # This is the first time calling final_touch
+            push!(bridge.bounds, ret)
+        elseif bridge.bounds[i] == ret
+            # We've called final_touch before, and the bounds match. No need to
+            # reformulate a second time.
+            continue
+        elseif bridge.bounds[i] != ret
+            # There is a stored bound, and the current bounds do not match. This
+            # means the model has been modified since the previous call to
+            # final_touch. We need to delete the bridge and start again.
+            MOI.delete(model, bridge)
+            MOI.Bridges.final_touch(bridge, model)
+            break
+        end
         unit_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         convex_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         for xi in ret[1]:ret[2]
@@ -277,9 +295,11 @@ function MOI.Bridges.final_touch(
     bridge::CountBelongsToMILPBridge{T,F},
     model::MOI.ModelLike,
 ) where {T,F}
-    MOI.delete(model, bridge)
     scalars = collect(MOI.Utilities.eachscalar(bridge.f))
     S, ci = _unit_expansion(bridge, model, scalars[2:end])
+    if isempty(S) && isempty(ci)
+        return # Nothing to bridge. We must have already called final_touch.
+    end
     append!(bridge.equal_to, ci)
     for (_, s) in S
         append!(bridge.variables, s)

diff --git a/src/Bridges/Constraint/bridges/count_distinct.jl b/src/Bridges/Constraint/bridges/count_distinct.jl
@@ -78,6 +78,7 @@ mutable struct CountDistinctToMILPBridge{
     less_than::Vector{
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.LessThan{T}},
     }
+    bounds::Vector{NTuple{2,T}}
     function CountDistinctToMILPBridge{T}(
         f::Union{MOI.VectorOfVariables,MOI.VectorAffineFunction{T}},
     ) where {T}
@@ -86,6 +87,7 @@ mutable struct CountDistinctToMILPBridge{
             MOI.VariableIndex[],
             MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}}[],
             MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.LessThan{T}}[],
+            NTuple{2,T}[],
         )
     end
 end
@@ -164,6 +166,7 @@ function MOI.delete(model::MOI.ModelLike, bridge::CountDistinctToMILPBridge)
         MOI.delete(model, x)
     end
     empty!(bridge.variables)
+    empty!(bridge.bounds)
     return
 end
 
@@ -266,8 +269,6 @@ function MOI.Bridges.final_touch(
     bridge::CountDistinctToMILPBridge{T,F},
     model::MOI.ModelLike,
 ) where {T,F}
-    # Clear any existing reformulations!
-    MOI.delete(model, bridge)
     S = Dict{T,Vector{MOI.VariableIndex}}()
     scalars = collect(MOI.Utilities.eachscalar(bridge.f))
     bounds = Dict{MOI.VariableIndex,NTuple{2,T}}()
@@ -280,6 +281,21 @@ function MOI.Bridges.final_touch(
                 "in the function has a non-finite domain: $x",
             )
         end
+        if length(bridge.bounds) < i - 1
+            # This is the first time calling final_touch
+            push!(bridge.bounds, ret)
+        elseif bridge.bounds[i-1] == ret
+            # We've called final_touch before, and the bounds match. No need to
+            # reformulate a second time.
+            continue
+        elseif bridge.bounds[i-1] != ret
+            # There is a stored bound, and the current bounds do not match. This
+            # means the model has been modified since the previous call to
+            # final_touch. We need to delete the bridge and start again.
+            MOI.delete(model, bridge)
+            MOI.Bridges.final_touch(bridge, model)
+            return
+        end
         unit_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         convex_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         for xi in ret[1]::T:ret[2]::T
@@ -306,6 +322,9 @@ function MOI.Bridges.final_touch(
             MOI.add_constraint(model, convex_f, MOI.EqualTo(one(T))),
         )
     end
+    if isempty(S)
+        return  # Nothing to bridge. We must have already called final_touch.
+    end
     count_terms = MOI.ScalarAffineTerm{T}[]
     # We use a sort so that the model order is deterministic.
     for s in sort!(collect(keys(S)))

diff --git a/src/Bridges/Constraint/bridges/count_distinct_reif.jl b/src/Bridges/Constraint/bridges/count_distinct_reif.jl
@@ -90,6 +90,7 @@ mutable struct ReifiedCountDistinctToMILPBridge{
     less_than::Vector{
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.LessThan{T}},
     }
+    bounds::Vector{NTuple{2,T}}
     function ReifiedCountDistinctToMILPBridge{T}(
         f::Union{MOI.VectorOfVariables,MOI.VectorAffineFunction{T}},
     ) where {T}
@@ -98,6 +99,7 @@ mutable struct ReifiedCountDistinctToMILPBridge{
             MOI.VariableIndex[],
             MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}}[],
             MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.LessThan{T}}[],
+            NTuple{2,T}[],
         )
     end
 end
@@ -179,6 +181,7 @@ function MOI.delete(
         MOI.delete(model, x)
     end
     empty!(bridge.variables)
+    empty!(bridge.bounds)
     return
 end
 
@@ -284,8 +287,6 @@ function MOI.Bridges.final_touch(
     bridge::ReifiedCountDistinctToMILPBridge{T,F},
     model::MOI.ModelLike,
 ) where {T,F}
-    # Clear any existing reformulations!
-    MOI.delete(model, bridge)
     S = Dict{T,Vector{MOI.VariableIndex}}()
     scalars = collect(MOI.Utilities.eachscalar(bridge.f))
     bounds = Dict{MOI.VariableIndex,NTuple{2,T}}()
@@ -298,6 +299,21 @@ function MOI.Bridges.final_touch(
                 "element $i in the function has a non-finite domain: $x",
             )
         end
+        if length(bridge.bounds) < i - 2
+            # This is the first time calling final_touch
+            push!(bridge.bounds, ret)
+        elseif bridge.bounds[i-2] == ret
+            # We've called final_touch before, and the bounds match. No need to
+            # reformulate a second time.
+            continue
+        elseif bridge.bounds[i-2] != ret
+            # There is a stored bound, and the current bounds do not match. This
+            # means the model has been modified since the previous call to
+            # final_touch. We need to delete the bridge and start again.
+            MOI.delete(model, bridge)
+            MOI.Bridges.final_touch(bridge, model)
+            return
+        end
         unit_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         convex_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
         for xi in ret[1]::T:ret[2]::T
@@ -324,6 +340,9 @@ function MOI.Bridges.final_touch(
             MOI.add_constraint(model, convex_f, MOI.EqualTo(one(T))),
         )
     end
+    if isempty(S)
+        return  # Nothing to bridge. We must have already called final_touch.
+    end
     count_terms = MOI.ScalarAffineTerm{T}[]
     # We use a sort so that the model order is deterministic.
     for s in sort!(collect(keys(S)))

diff --git a/src/Bridges/Constraint/bridges/count_greater_than.jl b/src/Bridges/Constraint/bridges/count_greater_than.jl
@@ -38,6 +38,7 @@ struct CountGreaterThanToMILPBridge{
     equal_to::Vector{
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}},
     }
+    bounds::Vector{NTuple{2,T}}
 end
 
 const CountGreaterThanToMILP{T,OT<:MOI.ModelLike} =
@@ -55,6 +56,7 @@ function bridge_constraint(
         MOI.VariableIndex[],
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.GreaterThan{T}}[],
         MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},MOI.EqualTo{T}}[],
+        NTuple{2,T}[],
     )
 end
 
@@ -112,6 +114,7 @@ function MOI.delete(model::MOI.ModelLike, bridge::CountGreaterThanToMILPBridge)
     empty!(bridge.equal_to)
     MOI.delete.(model, bridge.variables)
     empty!(bridge.variables)
+    empty!(bridge.bounds)
     return
 end
 
@@ -222,6 +225,7 @@ function _add_unit_expansion(
     S,
     bounds,
     x,
+    i,
 ) where {T,F}
     ret = _get_bounds(bridge, model, bounds, x)
     if ret === nothing
@@ -230,13 +234,28 @@ function _add_unit_expansion(
             "function has a non-finite domain: $x",
         )
     end
+    if length(bridge.bounds) < i
+        # This is the first time calling final_touch
+        push!(bridge.bounds, ret)
+    elseif bridge.bounds[i] == ret
+        # We've called final_touch before, and the bounds match. No need to
+        # reformulate a second time.
+        return
+    elseif bridge.bounds[i] != ret
+        # There is a stored bound, and the current bounds do not match. This
+        # means the model has been modified since the previous call to
+        # final_touch. We need to delete the bridge and start again.
+        MOI.delete(model, bridge)
+        MOI.Bridges.final_touch(bridge, model)
+        return
+    end
     unit_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
     convex_f = MOI.ScalarAffineFunction(MOI.ScalarAffineTerm{T}[], zero(T))
     for xi in ret[1]::T:ret[2]::T
         new_var, _ = MOI.add_constrained_variable(model, MOI.ZeroOne())
         push!(bridge.variables, new_var)
         if !haskey(S, xi)
-            S[xi] = Tuple{Float64,MOI.VariableIndex}[]
+            S[xi] = Tuple{T,MOI.VariableIndex}[]
         end
         push!(S[xi], new_var)
         push!(unit_f.terms, MOI.ScalarAffineTerm(T(-xi), new_var))
@@ -262,15 +281,13 @@ function MOI.Bridges.final_touch(
     bridge::CountGreaterThanToMILPBridge{T,F},
     model::MOI.ModelLike,
 ) where {T,F}
-    # Clear any existing reformulations!
-    MOI.delete(model, bridge)
     Sx = Dict{T,Vector{MOI.VariableIndex}}()
     Sy = Dict{T,Vector{MOI.VariableIndex}}()
     scalars = collect(MOI.Utilities.eachscalar(bridge.f))
     bounds = Dict{MOI.VariableIndex,NTuple{2,T}}()
-    _add_unit_expansion(bridge, model, Sy, bounds, scalars[2])
+    _add_unit_expansion(bridge, model, Sy, bounds, scalars[2], 1)
     for i in 3:length(scalars)
-        _add_unit_expansion(bridge, model, Sx, bounds, scalars[i])
+        _add_unit_expansion(bridge, model, Sx, bounds, scalars[i], i - 1)
     end
     # We use a sort so that the model order is deterministic.
     for s in sort!(collect(keys(Sy)))