[Bridges] fix BoundAlreadySet errors (#2376)

src/Bridges/bridge_optimizer.jl

@@ -697,6 +697,7 @@ function MOI.delete(b::AbstractBridgeOptimizer, ci::MOI.ConstraintIndex)
     else
         MOI.delete(b.model, ci)
     end
+    return
 end
 
 function MOI.delete(
@@ -1732,28 +1733,71 @@ function add_bridged_constraint(b, BridgeType, f, s)
     return ci
 end
 
+function _throw_if_bound_already_set(b, x, ::S2, ::Type{S1}) where {S1,S2}
+    ErrorType = _bound_error_type(S1, S2)
+    if ErrorType === nothing
+        return
+    end
+    ci = MOI.ConstraintIndex{MOI.VariableIndex,S1}(x.value)
+    if (is_bridged(b, S1) && MOI.is_valid(b, ci)) || MOI.is_valid(b.model, ci)
+        throw(ErrorType(x))
+    end
+    return
+end
+
+function _bound_error_type(
+    ::Type{S1},
+    ::Type{S2},
+) where {S1<:MOI.LessThan,S2<:MOI.LessThan}
+    return MOI.UpperBoundAlreadySet{S1,S2}
+end
+
+function _bound_error_type(::Type{S1}, ::Type{S2}) where {S1<:MOI.LessThan,S2}
+    return MOI.UpperBoundAlreadySet{S1,S2}
+end
+
+function _bound_error_type(::Type{S1}, ::Type{S2}) where {S1,S2<:MOI.LessThan}
+    return MOI.UpperBoundAlreadySet{S1,S2}
+end
+
+function _bound_error_type(::Type{S1}, ::Type{S2}) where {S1,S2}
+    return MOI.LowerBoundAlreadySet{S1,S2}
+end
+
+_bound_error_type(::Type{<:MOI.LessThan}, ::Type{<:MOI.GreaterThan}) = nothing
+
+_bound_error_type(::Type{<:MOI.GreaterThan}, ::Type{<:MOI.LessThan}) = nothing
+
 function _check_double_single_variable(
     b::AbstractBridgeOptimizer,
-    func::MOI.VariableIndex,
-    set,
-)
-    if !is_bridged(b, typeof(set))
-        # The variable might have been constrained in `b.model` to a set of type
-        # `typeof(set)` on creation.
-        ci = MOI.ConstraintIndex{MOI.VariableIndex,typeof(set)}(func.value)
-        if MOI.is_valid(b.model, ci)
-            error(
-                "The variable `$(func)` was constrained on creation ",
-                "to a set of type `$(typeof(set))`. Therefore, a ",
-                "`VariableIndex`-in-`$(typeof(set))` cannot be added on this ",
-                "variable. Use `MOI.set` with `MOI.ConstraintSet()` instead.",
-            )
-        end
-    end
+    x::MOI.VariableIndex,
+    s::S,
+) where {
+    T,
+    S<:Union{
+        MOI.Interval{T},
+        MOI.EqualTo{T},
+        MOI.Semicontinuous{T},
+        MOI.Semiinteger{T},
+        MOI.LessThan{T},
+        MOI.GreaterThan{T},
+    },
+}
+    # !!! warning
+    #     The order here is _very_ important because an Interval constraint
+    #     might get re-written into a LessThan and GreaterThan. To throw the
+    #     appropriate error, we _must_ check sets like `Interval` and `EqualTo`
+    #     _before_ `LessThan` and `GreaterThan`.
+    _throw_if_bound_already_set(b, x, s, MOI.Interval{T})
+    _throw_if_bound_already_set(b, x, s, MOI.EqualTo{T})
+    _throw_if_bound_already_set(b, x, s, MOI.Semicontinuous{T})
+    _throw_if_bound_already_set(b, x, s, MOI.Semiinteger{T})
+    _throw_if_bound_already_set(b, x, s, MOI.LessThan{T})
+    _throw_if_bound_already_set(b, x, s, MOI.GreaterThan{T})
     return
 end
 
-_check_double_single_variable(b::AbstractBridgeOptimizer, func, set) = nothing
+_check_double_single_variable(::AbstractBridgeOptimizer, ::Any, ::Any) = nothing
 
 function MOI.add_constraint(
     b::AbstractBridgeOptimizer,

src/Utilities/variables_container.jl

@@ -308,6 +308,8 @@ function MOI.is_valid(
     return !iszero(b.set_mask[ci.value] & _single_variable_flag(S))
 end
 
+MOI.is_valid(::VariablesContainer, ::MOI.ConstraintIndex) = false
+
 function MOI.get(
     model::VariablesContainer,
     ::MOI.ConstraintFunction,

test/Bridges/lazy_bridge_optimizer.jl

@@ -303,17 +303,8 @@ function test_MOI_runtests_LPModel()
     bridged = MOI.Bridges.full_bridge_optimizer(model, Float64)
     MOI.Test.runtests(
         bridged,
-        MOI.Test.Config(exclude = Any[MOI.optimize!]),
+        MOI.Test.Config(exclude = Any[MOI.optimize!]);
         include = ["test_model_", "test_constraint_"],
-        exclude = [
-            "test_constraint_ConstraintDualStart",
-            "test_constraint_ConstraintPrimalStart",
-            "test_model_default_DualStatus",
-            "test_model_default_PrimalStatus",
-            "test_model_default_TerminationStatus",
-            "test_model_LowerBoundAlreadySet",
-            "test_model_UpperBoundAlreadySet",
-        ],
     )
     return
 end
@@ -327,11 +318,24 @@ function test_MOI_runtests_StandardSDPAModel()
             exclude = Any[MOI.optimize!, MOI.SolverName, MOI.SolverVersion],
         );
         exclude = String[
-            "test_model_ListOfVariablesWithAttributeSet",
+            # TODO(odow): investigate. This seems like an actual bug.
+            "test_model_delete",
+            # 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",
+            # MOI.ScalarFunctionConstantNotZero is thrown, not of the original
+            # constraint, but of the bridged constraint. This seems okay. The
+            # fix would require that a bridge optimizer has a try-catch for this
+            # error.
             "test_model_ScalarFunctionConstantNotZero",
-            "test_model_delete",
+            # The error is:
+            # Cannot substitute `MOI.VariableIndex(1)` as it is bridged into `0.0 + 1.0 MOI.VariableIndex(-1)`.
+            # This seems okay. We can't get a list of variables if they are
+            # bridged.
+            "test_model_ListOfVariablesWithAttributeSet",
+
         ],
     )
     return
@@ -344,11 +348,7 @@ function test_MOI_runtests_GeometricSDPAModel()
         model,
         MOI.Test.Config(
             exclude = Any[MOI.optimize!, MOI.SolverName, MOI.SolverVersion],
-        );
-        exclude = String[
-            "test_model_LowerBoundAlreadySet",
-            "test_model_UpperBoundAlreadySet",
-        ],
+        ),
     )
     return
 end