Run all tests against bridged SDPA model (#2357)

* Run all tests against bridged SDPA model * Exclude failing tests for now * [Bridges] fix BoundAlreadySet errors (#2376) * Fix format * Update --------- Co-authored-by: odow <[email protected]> Co-authored-by: Oscar Dowson <[email protected]>
jump-dev · Dec 23, 2023 · ede6d1c · ede6d1c
1 parent b82d161
commit ede6d1c
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 39 deletions.
diff --git a/src/Bridges/bridge_optimizer.jl b/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,

diff --git a/src/Utilities/variables_container.jl b/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,

diff --git a/test/Bridges/lazy_bridge_optimizer.jl b/test/Bridges/lazy_bridge_optimizer.jl
@@ -104,7 +104,15 @@ end
 function MOI.supports(
     ::StandardLPModel{T},
     ::MOI.ObjectiveFunction{
-        <:Union{MOI.VariableIndex,MOI.ScalarQuadraticFunction{T}},
+        <:Union{
+            MOI.VariableIndex,
+            MOI.ScalarQuadraticFunction{T},
+            MOI.ScalarNonlinearFunction,
+            MOI.VectorOfVariables,
+            MOI.VectorAffineFunction{T},
+            MOI.VectorQuadraticFunction{T},
+            MOI.VectorNonlinearFunction,
+        },
     },
 ) where {T}
     return false
@@ -295,39 +303,49 @@ 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
 
 function test_MOI_runtests_StandardSDPAModel()
-    model = StandardSDPAModel{Float64}()
-    bridged = MOI.Bridges.full_bridge_optimizer(model, Float64)
+    model =
+        MOI.instantiate(StandardSDPAModel{Float64}; with_bridge_type = Float64)
     MOI.Test.runtests(
-        bridged,
-        MOI.Test.Config(exclude = Any[MOI.optimize!]),
-        include = ["ConstraintName", "VariableName"],
+        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",
+            # 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",
+            # 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
 end
 
 function test_MOI_runtests_GeometricSDPAModel()
-    model = GeometricSDPAModel{Float64}()
-    bridged = MOI.Bridges.full_bridge_optimizer(model, Float64)
+    model =
+        MOI.instantiate(GeometricSDPAModel{Float64}; with_bridge_type = Float64)
     MOI.Test.runtests(
-        bridged,
-        MOI.Test.Config(exclude = Any[MOI.optimize!]),
-        include = ["ConstraintName", "VariableName"],
+        model,
+        MOI.Test.Config(
+            exclude = Any[MOI.optimize!, MOI.SolverName, MOI.SolverVersion],
+        ),
     )
     return
 end

diff --git a/test/Bridges/sdpa_models.jl b/test/Bridges/sdpa_models.jl
@@ -62,7 +62,15 @@ end
 function MOI.supports(
     ::StandardSDPAModel{T},
     ::MOI.ObjectiveFunction{
-        <:Union{MOI.VariableIndex,MOI.ScalarQuadraticFunction{T}},
+        <:Union{
+            MOI.VariableIndex,
+            MOI.ScalarQuadraticFunction{T},
+            MOI.ScalarNonlinearFunction,
+            MOI.VectorOfVariables,
+            MOI.VectorAffineFunction{T},
+            MOI.VectorQuadraticFunction{T},
+            MOI.VectorNonlinearFunction,
+        },
     },
 ) where {T}
     return false