diff --git a/src/Bridges/bridge_optimizer.jl b/src/Bridges/bridge_optimizer.jl index 67b5661732..1405be62ac 100644 --- 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 index 585f85492e..18c65b9226 100644 --- 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 index 91ec43a005..43003c069c 100644 --- a/test/Bridges/lazy_bridge_optimizer.jl +++ b/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