Skip to content

[Bridges] Throw for bound already set on bridged variable #2383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 71 additions & 2 deletions src/Bridges/Variable/map.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ mutable struct Map <: AbstractDict{MOI.VariableIndex,AbstractBridge}
# `(ci::ConstraintIndex{MOI.VectorOfVariables}).value` ->
# the dimension of the set
vector_of_variables_length::Vector{Int64}
# Same as in `MOI.Utilities.VariablesContainer`
set_mask::Vector{UInt16}
end

function Map()
Expand All @@ -58,6 +60,7 @@ function Map()
Dict{MOI.ConstraintIndex,Int64}(),
Int64[],
Int64[],
UInt16[],
)
end

Expand Down Expand Up @@ -88,6 +91,7 @@ function Base.empty!(map::Map)
empty!(map.constraint_context)
empty!(map.vector_of_variables_map)
empty!(map.vector_of_variables_length)
empty!(map.set_mask)
return map
end

Expand All @@ -113,8 +117,9 @@ end
function Base.delete!(map::Map, vi::MOI.VariableIndex)
if iszero(map.info[-vi.value])
# Delete scalar variable
map.bridges[bridge_index(map, vi)] = nothing
map.sets[bridge_index(map, vi)] = nothing
index = bridge_index(map, vi)
map.bridges[index] = nothing
map.sets[index] = nothing
elseif has_keys(map, [vi])
# Delete whole vector
delete!(map, [vi])
Expand All @@ -131,6 +136,7 @@ function Base.delete!(map::Map, vi::MOI.VariableIndex)
map.index_in_vector[i] -= 1
end
end
map.set_mask[-vi.value] = MOI.Utilities._DELETED_VARIABLE
map.index_in_vector[-vi.value] = -1
return map
end
Expand All @@ -144,6 +150,7 @@ function Base.delete!(map::Map, vis::Vector{MOI.VariableIndex})
)
end
for vi in vis
map.set_mask[-vi.value] = MOI.Utilities._DELETED_VARIABLE
map.index_in_vector[-vi.value] = -1
end
map.bridges[bridge_index(map, first(vis))] = nothing
Expand Down Expand Up @@ -274,6 +281,64 @@ function MOI.is_valid(
map.sets[index] === S
end

"""
MOI.add_constraint(map::Map, vi::MOI.VariableIndex, set::MOI.AbstractScalarSet)

Record that a constraint `vi`-in-`set` is added and throws if a lower or upper bound
is set by this constraint and such bound has already been set for `vi`.
"""
function MOI.add_constraint(::Map, ::MOI.VariableIndex, ::MOI.AbstractScalarSet)
# Nothing to do as this is is not recognized as setting a lower or upper bound
end

# We cannot use `SUPPORTED_VARIABLE_SCALAR_SETS` because
# `Integer` and `ZeroOne` do not define `T` and we need `T`
# for `_throw_if_lower_bound_set`.
const _BOUNDED_VARIABLE_SCALAR_SETS{T} = Union{
MOI.EqualTo{T},
MOI.GreaterThan{T},
MOI.LessThan{T},
MOI.Interval{T},
MOI.Semicontinuous{T},
MOI.Semiinteger{T},
MOI.Parameter{T},
}

function MOI.add_constraint(
map::Map,
vi::MOI.VariableIndex,
::S,
) where {T,S<:_BOUNDED_VARIABLE_SCALAR_SETS{T}}
flag = MOI.Utilities._single_variable_flag(S)
index = -vi.value
mask = map.set_mask[index]
MOI.Utilities._throw_if_lower_bound_set(vi, S, mask, T)
MOI.Utilities._throw_if_upper_bound_set(vi, S, mask, T)
map.set_mask[index] = mask | flag
return
end

"""
delete(map::Map, ci::MOI.ConstraintIndex{MOI.VariableIndex,<:MOI.AbstractScalarSet})

Record that the constraint `vi`-in-`S` is deleted.
"""
function MOI.delete(
::Map,
ci::MOI.ConstraintIndex{MOI.VariableIndex,<:MOI.AbstractScalarSet},
)
# Nothing to do as this is is not recognized as setting a lower or upper bound
end

function MOI.delete(
map::Map,
ci::MOI.ConstraintIndex{MOI.VariableIndex,S},
) where {T,S<:_BOUNDED_VARIABLE_SCALAR_SETS{T}}
flag = MOI.Utilities._single_variable_flag(S)
map.set_mask[-ci.value] &= ~flag
return
end

"""
constraints_with_set(map::Map, S::Type{<:MOI.AbstractSet})

Expand Down Expand Up @@ -384,6 +449,7 @@ function add_key_for_bridge(
push!(map.index_in_vector, 0)
push!(map.bridges, nothing)
push!(map.sets, typeof(set))
push!(map.set_mask, 0x0000)
map.bridges[bridge_index] = call_in_context(map, bridge_index, bridge_fun)
index = -bridge_index
variable = MOI.VariableIndex(index)
Expand All @@ -400,6 +466,7 @@ function add_key_for_bridge(
end
end
end
MOI.add_constraint(map, variable, set)
return variable, MOI.ConstraintIndex{MOI.VariableIndex,typeof(set)}(index)
end

Expand Down Expand Up @@ -443,12 +510,14 @@ function add_keys_for_bridge(
push!(map.index_in_vector, 1)
push!(map.bridges, nothing)
push!(map.sets, typeof(set))
push!(map.set_mask, 0x0000)
for i in 2:MOI.dimension(set)
push!(map.parent_index, 0)
push!(map.info, i)
push!(map.index_in_vector, i)
push!(map.bridges, nothing)
push!(map.sets, nothing)
push!(map.set_mask, 0x0000)
end
map.bridges[bridge_index] = call_in_context(map, bridge_index, bridge_fun)
variables = MOI.VariableIndex[
Expand Down
11 changes: 10 additions & 1 deletion src/Bridges/bridge_optimizer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,10 @@ function MOI.delete(b::AbstractBridgeOptimizer, vi::MOI.VariableIndex)
return
end

function MOI.delete(b::AbstractBridgeOptimizer, ci::MOI.ConstraintIndex)
function MOI.delete(
b::AbstractBridgeOptimizer,
ci::MOI.ConstraintIndex{F},
) where {F}
if is_bridged(b, ci)
MOI.throw_if_not_valid(b, ci)
br = bridge(b, ci)
Expand All @@ -687,6 +690,11 @@ function MOI.delete(b::AbstractBridgeOptimizer, ci::MOI.ConstraintIndex)
else
delete!(Constraint.bridges(b)::Constraint.Map, ci)
end
if F === MOI.VariableIndex && ci.value < 0
# Constraint on a bridged variable so we need to remove the flag
# if it is a bound
MOI.delete(Variable.bridges(b), ci)
end
Variable.call_in_context(
Variable.bridges(b),
ci,
Expand Down Expand Up @@ -1838,6 +1846,7 @@ function MOI.add_constraint(
typeof(f),
typeof(s),
)
MOI.add_constraint(Variable.bridges(b), f, s)
return add_bridged_constraint(b, BridgeType, f, s)
end
elseif f isa MOI.VectorOfVariables
Expand Down
15 changes: 15 additions & 0 deletions test/Bridges/Variable/map.jl
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,21 @@ function test_EmptyMap()
return
end

function test_double_variable_bound()
map = MOI.Bridges.Variable.Map()
b1 = VariableDummyBridge(1)
set1 = MOI.EqualTo(0.0)
v1, c1 = MOI.Bridges.Variable.add_key_for_bridge(map, () -> b1, set1)
MOI.is_valid(map, c1)
MOI.add_constraint(map, v1, MOI.Integer())
cint = MOI.ConstraintIndex{typeof(v1),MOI.Integer}(v1.value)
MOI.delete(map, cint)
set2 = MOI.LessThan(1.0)
err = MOI.UpperBoundAlreadySet{typeof(set1),typeof(set2)}(v1)
@test_throws err MOI.add_constraint(map, v1, set2)
return
end

end # module

TestVariableMap.runtests()
9 changes: 1 addition & 8 deletions test/Bridges/lazy_bridge_optimizer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -316,14 +316,7 @@ function test_MOI_runtests_StandardSDPAModel()
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",
],
),
)
return
end
Expand Down