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

[FileFormats] read double-sided variable bounds separately #2548

Merged
merged 3 commits into from
Oct 7, 2024
Merged

[FileFormats] read double-sided variable bounds separately #2548

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad3c37f
[FileFormats] read double-sided variable bounds separately
odow Oct 7, 2024
8deb004
Update test/FileFormats/MPS/MPS.jl
odow Oct 7, 2024
1b236e4
Add comment
odow Oct 7, 2024
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
5 changes: 4 additions & 1 deletion src/FileFormats/LP/LP.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,10 @@ function _parse_section(
return
elseif -Inf < lb < ub < Inf
_delete_default_lower_bound_if_present(model, cache, x)
MOI.add_constraint(model, x, MOI.Interval(lb, ub))
# Do not add MOI.Interval constraints because we want to follow
# JuMP's convention of adding separate lower and upper bounds.
MOI.add_constraint(model, x, MOI.GreaterThan(lb))
MOI.add_constraint(model, x, MOI.LessThan(ub))
return
elseif lb == -Inf
_delete_default_lower_bound_if_present(model, cache, x)
Expand Down
5 changes: 4 additions & 1 deletion src/FileFormats/MPS/MPS.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1287,7 +1287,10 @@ function _add_variable(model, data, variable_map, i, name)
variable_map[name] = x
MOI.set(model, MOI.VariableName(), x, name)
set = bounds_to_set(data.col_lower[i], data.col_upper[i])
if set !== nothing
if set isa MOI.Interval
MOI.add_constraint(model, x, MOI.GreaterThan(set.lower::Float64))
MOI.add_constraint(model, x, MOI.LessThan(set.upper::Float64))
elseif set !== nothing
MOI.add_constraint(model, x, set)
end
Copy link

Choose a reason for hiding this comment

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

Sorry if this is too nit-picky, but: Could creating the "helper" Interval set be an overhead for large files?

Something like this would be possible here (but makes it more convoluted):

if isfinite(data.col_lower[i])
    MOI.add_constraint(model, x, MOI.GreaterThan(data.col_lower[i]::Float64))
end

if isfinite(data.col_upper[i])
    MOI.add_constraint(model, x, MOI.LessThan(data.col_upper[i]::Float64))
end

This would also handle two edge cases differently:

  1. If data.col_lower[i] > data.col_upper[i], it adds the bounds keeping the model infeasible (bounds_to_set would currently remove the bounds and create a free variable)
  2. If data.col_lower[i] == data.col_upper[i], it sticks to adding bounds, and does not convert those into a single equality

(1.) is maybe not really important, but (2.) could lead to similar problems as the initial version:

  • I create a variable that somewhere along the way gets equal bounds, then
  • write that to a file, read it back in, and
  • the bounds are gone, has_*_bound will return false, and I can't change the bounds anymore.

Further, this also impacts how fix(x, value; force=false) behaves. With bounds it errors, with the EqualTo it overwrites.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could creating the "helper" Interval set be an overhead for large files?

Nope. It's a struct that is on the stack.

Something like this would be possible here

bounds_to_set already does this 😄 it will return an Interval only if both sides are finite.

the bounds are gone, has_*_bound will return false, and I can't change the bounds anymore.

You'd need to unfix(x); set_lower_bound(x, lb).

The issue is that JuMP doesn't offer a nice way to interact with interval bounds.

I agree that lower/upper/fixed bounds are more convoluted than a plain lower/upper bound. You'll need to write some JuMP code to deal with the different situations.

if data.vtype[i] == VTYPE_INTEGER
Expand Down
19 changes: 15 additions & 4 deletions test/FileFormats/LP/LP.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ module TestLP
using Test

import MathOptInterface as MOI
const LP = MOI.FileFormats.LP
import MathOptInterface.FileFormats: LP

const LP_TEST_FILE = "test.lp"

function test_show()
Expand Down Expand Up @@ -464,7 +465,8 @@ function test_read_example_lo1()
@test (MOI.ScalarAffineFunction{Float64}, MOI.LessThan{Float64}) in
constraints
@test (MOI.VariableIndex, MOI.GreaterThan{Float64}) in constraints
@test (MOI.VariableIndex, MOI.Interval{Float64}) in constraints
@test (MOI.VariableIndex, MOI.LessThan{Float64}) in constraints
@test !((MOI.VariableIndex, MOI.Interval{Float64}) in constraints)
io = IOBuffer()
write(io, model)
seekstart(io)
Expand Down Expand Up @@ -525,7 +527,8 @@ function test_read_model1()
@test (MOI.ScalarAffineFunction{Float64}, MOI.LessThan{Float64}) in
constraints
@test (MOI.VariableIndex, MOI.GreaterThan{Float64}) in constraints
@test (MOI.VariableIndex, MOI.Interval{Float64}) in constraints
@test (MOI.VariableIndex, MOI.LessThan{Float64}) in constraints
@test !((MOI.VariableIndex, MOI.Interval{Float64}) in constraints)
@test (MOI.VariableIndex, MOI.Integer) in constraints
@test (MOI.VariableIndex, MOI.ZeroOne) in constraints
@test (MOI.VectorOfVariables, MOI.SOS1{Float64}) in constraints
Expand All @@ -543,9 +546,17 @@ function test_read_model2()
@test (MOI.ScalarAffineFunction{Float64}, MOI.LessThan{Float64}) in
constraints
@test (MOI.VariableIndex, MOI.GreaterThan{Float64}) in constraints
@test (MOI.VariableIndex, MOI.Interval{Float64}) in constraints
@test (MOI.VariableIndex, MOI.LessThan{Float64}) in constraints
@test !((MOI.VariableIndex, MOI.Interval{Float64}) in constraints)
@test (MOI.VariableIndex, MOI.Integer) in constraints
@test (MOI.VariableIndex, MOI.ZeroOne) in constraints
@test MOI.get(model, MOI.VariableName(), MOI.VariableIndex(2)) == "V5"
ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.LessThan{Float64}}(2)
@test MOI.get(model, MOI.ConstraintSet(), ci) == MOI.LessThan(1.0)
ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.GreaterThan{Float64}}(2)
@test MOI.get(model, MOI.ConstraintSet(), ci) == MOI.GreaterThan(0.0)
ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.Interval{Float64}}(2)
@test !MOI.is_valid(model, ci)
@test MOI.get(model, MOI.VariableName(), MOI.VariableIndex(8)) == "V8"
@test model.variables.lower[8] == -Inf
@test model.variables.upper[8] == -3
Expand Down
18 changes: 13 additions & 5 deletions test/FileFormats/MPS/MPS.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ function test_stacked_data()
con3: 1.0 * x in Interval(3.0, 7.0)
con4: 2.0 * x in Interval(4.0, 8.0)
y in Integer()
y in Interval(1.0, 4.0)
y >= 1.0
y <= 4.0
z in ZeroOne()
""",
)
Expand All @@ -182,7 +183,8 @@ function test_stacked_data()
["blank_obj", "con1", "con2", "con3", "con4"],
[
("y", MOI.Integer()),
("y", MOI.Interval{Float64}(1.0, 4.0)),
("y", MOI.GreaterThan{Float64}(1.0)),
("y", MOI.LessThan{Float64}(4.0)),
("z", MOI.ZeroOne()),
],
)
Expand All @@ -193,8 +195,12 @@ function test_integer_default_bounds()
model = MPS.Model()
MOI.read_from_file(model, joinpath(@__DIR__, "integer_default_bounds.mps"))
x = only(MOI.get(model, MOI.ListOfVariableIndices()))
ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.GreaterThan{Float64}}(x.value)
odow marked this conversation as resolved.
Show resolved Hide resolved
@test MOI.get(model, MOI.ConstraintSet(), ci) == MOI.GreaterThan(0.0)
ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.LessThan{Float64}}(x.value)
@test MOI.get(model, MOI.ConstraintSet(), ci) == MOI.LessThan(1.0)
ci = MOI.ConstraintIndex{MOI.VariableIndex,MOI.Interval{Float64}}(x.value)
@test MOI.get(model, MOI.ConstraintSet(), ci) == MOI.Interval(0.0, 1.0)
@test !MOI.is_valid(model, ci)
return
end

Expand Down Expand Up @@ -482,7 +488,8 @@ function test_nonzero_variable_bounds()
x == 1.0
y >= 2.0
z <= 3.0
w in Interval(4.0, 5.0)
w >= 4.0
w <= 5.0
""",
)
io = IOBuffer()
Expand All @@ -499,7 +506,8 @@ function test_nonzero_variable_bounds()
("x", MOI.EqualTo{Float64}(1.0)),
("y", MOI.GreaterThan{Float64}(2.0)),
("z", MOI.LessThan{Float64}(3.0)),
("w", MOI.Interval{Float64}(4.0, 5.0)),
("w", MOI.GreaterThan{Float64}(4.0)),
("w", MOI.LessThan{Float64}(5.0)),
],
)
return
Expand Down
Loading