Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Test support for VariablePrimalStart in Bridges.Variable #2116

Merged
merged 7 commits into from
Mar 14, 2023
Merged

Test support for VariablePrimalStart in Bridges.Variable #2116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ac319bf
WIP: test support for VariablePrimalStart in Bridges.Variable
odow Mar 9, 2023
6cfd6cb
Opt-out for RSOC/SOC bridges
odow Mar 9, 2023
967c9c7
Default supports
blegat Mar 13, 2023
770592d
False positive supports
blegat Mar 13, 2023
1c7d66e
Revert "Default supports"
blegat Mar 13, 2023
d4723b5
Fixes
blegat Mar 13, 2023
9465fa4
Fix format
blegat Mar 13, 2023
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
22 changes: 21 additions & 1 deletion src/Bridges/Bridges.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,13 @@ function _test_structural_identical(a::MOI.ModelLike, b::MOI.ModelLike)
end

"""
runtests(Bridge::Type{<:AbstractBridge}, input::String, output::String)
runtests(
Bridge::Type{<:AbstractBridge},
input::String,
output::String;
variable_start = 1.2,
constraint_start = 1.2,
)

Run a series of tests that check the correctness of `Bridge`.

Expand Down Expand Up @@ -246,6 +252,7 @@ function runtests(
Bridge::Type{<:AbstractBridge},
input::String,
output::String;
variable_start = 1.2,
constraint_start = 1.2,
)
# Load model and bridge it
Expand All @@ -263,6 +270,19 @@ function runtests(
target = MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}())
MOI.Utilities.loadfromstring!(target, output)
_test_structural_identical(target, inner)
# Test VariablePrimalStart
attr = MOI.VariablePrimalStart()
bridge_supported = all(values(Variable.bridges(model))) do bridge
return MOI.supports(model, attr, typeof(bridge))
end
if MOI.supports(model, attr, MOI.VariableIndex) && bridge_supported
x = MOI.get(model, MOI.ListOfVariableIndices())
MOI.set(model, attr, x, fill(nothing, length(x)))
Test.@test all(isnothing, MOI.get(model, attr, x))
primal_start = fill(constraint_start, length(x))
MOI.set(model, attr, x, primal_start)
Test.@test MOI.get(model, attr, x) ≈ primal_start
end
# Test ConstraintPrimalStart and ConstraintDualStart
for (F, S) in MOI.get(model, MOI.ListOfConstraintTypesPresent())
for ci in MOI.get(model, MOI.ListOfConstraintIndices{F,S}())
Expand Down
21 changes: 19 additions & 2 deletions src/Bridges/Variable/bridges/free.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,12 @@ function MOI.get(
i::MOI.Bridges.IndexInVector,
) where {T}
n = div(length(bridge.variables), 2)
return MOI.get(model, attr, bridge.variables[i.value]) -
MOI.get(model, attr, bridge.variables[n+i.value])
primal_pos = MOI.get(model, attr, bridge.variables[i.value])
primal_neg = MOI.get(model, attr, bridge.variables[n+i.value])
if primal_pos === nothing || primal_neg === nothing
return nothing
end
return primal_pos - primal_neg
end

function MOI.Bridges.bridged_function(
Expand Down Expand Up @@ -177,3 +181,16 @@ function MOI.set(
MOI.set(model, attr, bridge.variables[n+i.value], -min(zero(value), value))
return
end

function MOI.set(
model::MOI.ModelLike,
attr::MOI.VariablePrimalStart,
bridge::FreeBridge,
::Nothing,
i::MOI.Bridges.IndexInVector,
)
n = div(length(bridge.variables), 2)
MOI.set(model, attr, bridge.variables[i.value], nothing)
MOI.set(model, attr, bridge.variables[n+i.value], nothing)
return
end
9 changes: 9 additions & 0 deletions src/Bridges/Variable/bridges/rsoc_soc.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,12 @@ function MOI.Bridges.inverse_adjoint_map_function(
)
return MOI.Bridges.map_function(BT, func)
end

function MOI.supports(
::MOI.ModelLike,
::MOI.VariablePrimalStart,
::Type{<:RSOCtoSOCBridge},
)
# See https://github.com/jump-dev/MathOptInterface.jl/issues/2117
return false
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opted-out here, since setting or getting the start can't be done in isolation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is part of the optional API. By the way, this should be default as in

MathOptInterface.jl/src/Bridges/bridge.jl

Lines 105 to 149 in 5025b50

"""
MOI.supports(
model::MOI.ModelLike,
attr::MOI.AbstractConstraintAttribute,
BT::Type{<:AbstractBridge},
)
Return a `Bool` indicating whether `BT` supports setting `attr` to `model`.
"""
function MOI.supports(
::MOI.ModelLike,
::MOI.AbstractConstraintAttribute,
::Type{<:AbstractBridge},
)
return false
end
"""
function MOI.get(
model::MOI.ModelLike,
attr::MOI.AbstractConstraintAttribute,
bridge::AbstractBridge,
)
Return the value of the attribute `attr` of the model `model` for the
constraint bridged by `bridge`.
"""
function MOI.get(
::MOI.ModelLike,
attr::MOI.AbstractConstraintAttribute,
bridge::AbstractBridge,
)
return throw(
ArgumentError(
"Bridge of type `$(typeof(bridge))` does not support accessing " *
"the attribute `$attr`. If you encountered this error " *
"unexpectedly, it probably means your model has been " *
"reformulated using the bridge, and you are attempting to query " *
"an attribute that we haven't implemented yet for this bridge. " *
"Please open an issue at https://github.com/jump-dev/MathOptInterface.jl/issues/new " *
"and provide a reproducible example explaining what you were " *
"trying to do.",
),
)
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bridges should have to implement it to return false

Copy link
Member

@blegat blegat Mar 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opted-out here, since setting or getting the start can't be done in isolation.

I understand what you mean now, so I reverted back to your solution and opened #2117

9 changes: 9 additions & 0 deletions src/Bridges/Variable/bridges/soc_rsoc.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,12 @@ function MOI.Bridges.inverse_adjoint_map_function(
)
return MOI.Bridges.map_function(BT, func)
end

function MOI.supports(
odow marked this conversation as resolved.
Show resolved Hide resolved
::MOI.ModelLike,
::MOI.VariablePrimalStart,
::Type{<:SOCtoRSOCBridge},
)
# https://github.com/jump-dev/MathOptInterface.jl/issues/2117
return false
end
22 changes: 15 additions & 7 deletions src/Bridges/Variable/bridges/vectorize.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,24 @@ function MOI.get(
return value + bridge.set_constant
end

function MOI.get(
function MOI.supports(
model::MOI.ModelLike,
attr::MOI.VariablePrimalStart,
bridge::VectorizeBridge,
::Type{<:VectorizeBridge},
)
return MOI.get(model, attr, bridge.variable) + bridge.set_constant
return MOI.supports(model, attr, MOI.VariableIndex)
end

function MOI.supports(
function MOI.get(
model::MOI.ModelLike,
attr::MOI.VariablePrimalStart,
::Type{<:VectorizeBridge},
bridge::VectorizeBridge,
)
return MOI.supports(model, attr, MOI.VariableIndex)
start = MOI.get(model, attr, bridge.variable)
if start === nothing
return nothing
end
return start + bridge.set_constant
end

function MOI.set(
Expand All @@ -196,7 +200,11 @@ function MOI.set(
bridge::VectorizeBridge,
value,
)
MOI.set(model, attr, bridge.variable, value - bridge.set_constant)
if value === nothing
MOI.set(model, attr, bridge.variable, value)
else
MOI.set(model, attr, bridge.variable, value - bridge.set_constant)
end
return
end

Expand Down
11 changes: 9 additions & 2 deletions src/Bridges/Variable/set_map.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ function MOI.get(
i::MOI.Bridges.IndexInVector,
)
value = MOI.get(model, attr, bridge.variables)
if any(isnothing, value)
return nothing
end
return MOI.Bridges.map_function(typeof(bridge), value, i)
end

Expand All @@ -186,8 +189,12 @@ function MOI.set(
value,
i::MOI.Bridges.IndexInVector,
)
bridged_value = MOI.Bridges.inverse_map_function(typeof(bridge), value)
MOI.set(model, attr, bridge.variables[i.value], bridged_value)
if value === nothing
MOI.set(model, attr, bridge.variables[i.value], nothing)
else
bridged_value = MOI.Bridges.inverse_map_function(typeof(bridge), value)
MOI.set(model, attr, bridge.variables[i.value], bridged_value)
end
return
end

Expand Down
7 changes: 6 additions & 1 deletion src/Bridges/bridge_optimizer.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,9 @@ function MOI.get(
attr::MOI.AbstractVariableAttribute,
indices::Vector{MOI.VariableIndex},
)
if any(index -> is_bridged(b, index), indices)
# `Variable.has_bridges` is used as a shortcut to speedup in case no variable bridge is used
if Variable.has_bridges(Variable.bridges(b)) &&
any(index -> is_bridged(b, index), indices)
return MOI.get.(b, attr, indices)
else
return unbridged_function.(b, MOI.get(b.model, attr, indices))
Expand All @@ -1149,6 +1151,9 @@ function MOI.supports(
attr::MOI.AbstractVariableAttribute,
::Type{MOI.VariableIndex},
)
# `supports` should only return `false` in case `MOI.set` always errors.
# If some variables are bridged by bridges that do not support `attr`, we
# should therefore still return `true`.
return MOI.supports(b.model, attr, MOI.VariableIndex)
end

Expand Down