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.

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

Inconsistent "interval" constraints when reading models from file #2547

Closed
sstroemer opened this issue Oct 7, 2024 · 3 comments · Fixed by #2548
Closed

Inconsistent "interval" constraints when reading models from file #2547

sstroemer opened this issue Oct 7, 2024 · 3 comments · Fixed by #2548

Comments

@sstroemer
Copy link

Interval constraints in file readers seem to be handled differently when reading back a model (c.f. #2121, #2351). This happens for LP and MPS files. It "works" with MOF, and is weird with NL. I'm not sure right now if making these consistent is even possible with the file formats (it's 05:18am at my place so this might be wrong in many ways...).

MWE:

using JuMP

m = Model()
@variable(m, 0 <= x <= 1)
has_lower_bound(x)              # true

write_to_file(m, "test.lp")
m = read_from_file("test.lp")

x = all_variables(m)[1]
has_lower_bound(x)              # false

This may, e.g., lead to

has_lower_bound(x) || set_lower_bound(x, 0)

# ERROR: MathOptInterface.LowerBoundAlreadySet{MathOptInterface.Interval{Float64},
# MathOptInterface.GreaterThan{Float64}}: Cannot add `VariableIndex`-in-`MathOptInterface.GreaterThan{Float64}`
# constraint for variable MOI.VariableIndex(1) as a `VariableIndex`-in-`MathOptInterface.Interval{Float64}`
# constraint was already set for this variable and both constraints set a lower bound.

Additional information:

A model looking like:

Subject to
 x ≥ 0
 x ≤ 1

leads to:

  • LP: 0 <= x <= 1 in the bounds section
  • MPS: LO 0 and UP 1 in the bounds section
  • a correct NL file (as far as I can decipher...)

One that is constructed using an explicit interval:

Subject to
 x ∈ [0, 1]

leads to: exactly the same files for LP, MPS, and NL as with the non-interval model

I can't really say if the last point with the interval NL file is a bug or intended... I kinda think that AMPL supports lower- and upper-bounds on non-linear constraints - there is a type for that (?) and the NL writer seems to handle these here. But... this format always confuses me, so this is just guess work. But if that were correct, then the NL file is kinda inconsistent too.

@odow
Copy link
Member

odow commented Oct 7, 2024

This is expected (current) behavior, but I agree it's pretty subtle:

  • JuMP does not add Interval bounds. It adds separate GreaterThan and LessThan
  • has_lower_bound is a synonym for "does this have a GreaterThan bound"
  • The file formats read bounds as Interval
  • JuMP's has_lower_bound returns false because there is no GreaterThan bound

That's a pretty reasonable argument to parse variable bounds as a separate ones for LP, MPS, and NL files, which doesn't distinguish between individual bounds and Interval.

@sstroemer
Copy link
Author

One further question, and maybe the reason for my initial confusion:

has_lower_bound(x) may return false for a variable x, while set_lower_bound(x, value) throws an error (even detecting the set):

[...] as a `VariableIndex`-in-`MathOptInterface.Interval{Float64}` constraint was already set [...]

I resorted to using this

try; has_lower_bound(x) || set_lower_bound(x, value); catch; end

but that felt hacky.


Maybe (but that may be more of a JuMP than a MOI topic) a function that allows safely setting/changing variable bounds - or is there already one?

@odow
Copy link
Member

odow commented Oct 7, 2024

It's because JuMP doesn't use or have support for Interval constraints. has_lower_bound is really "do you have a GreaterThan" constraint. The answer is "no".

We assume that if you've added an Interval bound you're doing something special.

@odow odow closed this as completed in #2548 Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants