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

Rational infinite constraints do not work anymore #2894

Closed
schillic opened this issue Feb 27, 2022 · 5 comments · Fixed by #2896
Closed

Rational infinite constraints do not work anymore #2894

schillic opened this issue Feb 27, 2022 · 5 comments · Fixed by #2896

Comments

@schillic
Copy link
Contributor

I noticed the following behavioral change in #2618:
In previous versions you could use solvers like GLPK with Rational numbers and infinite bounds:

pkg> add GLPK@0.14, JuMP
  [60bf3e95] GLPK v0.14.14
  [4076af6c] JuMP v0.21.10

julia> using JuMP, GLPK
julia> N = Rational{Int64};
julia> l = N[0, 0];
julia> u = N[Inf, Inf];
julia> solver = JuMP.optimizer_with_attributes(() -> GLPK.Optimizer(method=GLPK.EXACT));
julia> model = Model(solver);
julia> @variable(model, l[i] <= x[i=1:2] <= u[i])
2-element Vector{VariableRef}:
 x[1]
 x[2]

After the change in #2618, the last command fails because the bounds are now replaced by NaN and then in the end a call to rationalize fails:

pkg> add GLPK@0.15.0, JuMP@0.22.0
  [60bf3e95] GLPK v0.15.0
  [4076af6c] JuMP v0.22.0

julia> ...run code...
ERROR: InexactError: Int64(NaN)
Stacktrace:
  [1] Int64
    @ ./float.jl:812 [inlined]
  [2] rationalize(#unused#::Type{Int64}, x::Float64, tol::Int64)
    @ Base ./rational.jl:162
  [3] #rationalize#218
    @ ./rational.jl:217 [inlined]
  [4] Rational
    @ ./rational.jl:120 [inlined]
  [5] convert
    @ ./number.jl:7 [inlined]
  [6] VariableInfo(has_lb::Bool, lower_bound::Rational{Int64}, has_ub::Bool, upper_bound::Rational{Int64}, has_fix::Bool, fixed_value::Float64, has_start::Bool, start::Float64, binary::Bool, integer::Bool)
    @ JuMP ~/.julia/packages/JuMP/zn6NT/src/variables.jl:141
  [7] #4
    @ ~/.julia/packages/JuMP/zn6NT/src/Containers/macro.jl:309 [inlined]
  [8] #37
    @ ~/.julia/packages/JuMP/zn6NT/src/Containers/container.jl:72 [inlined]
  [9] iterate
    @ ./generator.jl:47 [inlined]
 [10] collect(itr::Base.Generator{JuMP.Containers.VectorizedProductIterator{Tuple{Base.OneTo{Int64}}}, JuMP.Containers.var"#37#38"{var"#4#6"{Model, Vector{Rational{Int64}}, Vector{Rational{Int64}}}}})
    @ Base ./array.jl:724
 [11] map(f::Function, A::JuMP.Containers.VectorizedProductIterator{Tuple{Base.OneTo{Int64}}})
    @ Base ./abstractarray.jl:2878
 [12] container
    @ ~/.julia/packages/JuMP/zn6NT/src/Containers/container.jl:72 [inlined]
 [13] container(f::Function, indices::JuMP.Containers.VectorizedProductIterator{Tuple{Base.OneTo{Int64}}})
    @ JuMP.Containers ~/.julia/packages/JuMP/zn6NT/src/Containers/container.jl:66
 [14] macro expansion
    @ ~/.julia/packages/JuMP/zn6NT/src/macros.jl:142 [inlined]
 [15] top-level scope
    @ REPL[3]:6

Indeed:

julia> rationalize(NaN)
ERROR: InexactError: Int64(NaN)

I would like to ask whether you consider bringing back the support for Rational inputs or whether users should rather avoid such cases instead.

@odow
Copy link
Member

odow commented Feb 27, 2022

In previous versions you could use solvers like GLPK with Rational numbers and infinite bounds:

This is incorrect. We converted everything to Float64: #2025.

We got rid of infinite bounds

julia> using JuMP

julia> model = Model();

julia> @variable(model, 0 <= x <= Inf)
x

julia> has_upper_bound(x)
false

so you can specify them, but they won't be passed to the solver. I'd have to dig into why the rationalize is throwing that error instead of something more informative.

However, what does this even mean?

julia> Rational{Int}(Inf)
1//0

Even if you could pass GLPK rational coefficients, I don't think you can pass 1//0 and expect something meaningful.

@odow
Copy link
Member

odow commented Feb 27, 2022

Ah. The error is coming from

julia> convert(Rational{Int}, NaN)
ERROR: InexactError: Int64(NaN)
Stacktrace:
 [1] Int64
   @ ./float.jl:723 [inlined]
 [2] rationalize(#unused#::Type{Int64}, x::Float64, tol::Int64)
   @ Base ./rational.jl:162
 [3] #rationalize#184
   @ ./rational.jl:217 [inlined]
 [4] Rational
   @ ./rational.jl:120 [inlined]
 [5] convert(#unused#::Type{Rational{Int64}}, x::Float64)
   @ Base ./number.jl:7
 [6] top-level scope
   @ REPL[38]:1

This method is incorrect if NaN cannot be converted to T

JuMP.jl/src/variables.jl

Lines 118 to 136 in 9b4847a

function VariableInfo(
has_lb::Bool,
lower_bound::S,
has_ub::Bool,
upper_bound::T,
has_fix::Bool,
fixed_value::U,
has_start::Bool,
start::V,
binary::Bool,
integer::Bool,
) where {S,T,U,V}
if has_lb && !_isfinite(lower_bound)
has_lb = false
lower_bound = NaN
end
if has_ub && !_isfinite(upper_bound)
has_ub = false
upper_bound = NaN

@schillic
Copy link
Contributor Author

Thanks @odow for the quick fix! Do you mind releasing a new version?

@odow
Copy link
Member

odow commented Feb 28, 2022

Do you mind releasing a new version?

We'll probably wait a few more days before releasing 0.23.1. There are some in-progress PRs I'd like to get in first.

I'd encourage you to remove the Rational coefficients from your code in the mean-time.

@schillic
Copy link
Contributor Author

Sure, sounds good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants