jump-dev / JuMP.jl

Modeling language for Mathematical Optimization (linear, mixed-integer, conic, semidefinite, nonlinear)
http://jump.dev/JuMP.jl/
Other
2.17k stars 390 forks source link

Method error in value(::Function, ::Number) #3775

Closed mitchphillipson closed 1 week ago

mitchphillipson commented 1 week ago

Demonstration of Issue

I am using the value(::Function, ::jump_object) version of value. However, if I put a number in the second coordinate, I get an error.

using JuMP

M = Model()
@variable(M, x)

D = Dict(x => 1)

value( i -> get(D, i, missing), x) # Works as expected

value( i -> get(D, i, missing), 1) # Method Error

Suggested Fix

Add a method on value of the form

value(::Function, x::Number) = x

Why am I getting this error?

I am creating expressions that potentially collapse to a number. I need to evaluate these expressions at a specific point, hence the error.

odow commented 1 week ago

I understand the motivation, but this makes me go hmmmmmmmm.

julia> using JuMP

julia> model = Model();

julia> @variable(model, x, start = 2);

julia> @expression(model, expr[i in 0:2], (1 + x)^i)
1-dimensional DenseAxisArray{Any,1,...} with index sets:
    Dimension 1, 0:2
And data, a 3-element Vector{Any}:
 1.0
  x + 1
  x² + 2 x + 1

julia> value.(start_value, expr)
ERROR: MethodError: no method matching value(::typeof(start_value), ::Float64)
mitchphillipson commented 1 week ago

Yes, that feels like it should error. Do you have a suggested workaround? In my package I have essentially the code I suggested, which isn't the best.

I'm also moderately embarrassed I didn't think to just put start_value in value. Obviously that's better than building a dictionary.

mitchphillipson commented 1 week ago

I'm thinking about this more and value(F, 1) feels like it should still be 1. You're taking the value and the value is always 1, regardless of the function. Does this cause issues if F is some "weird" function?

odow commented 1 week ago

See https://github.com/jump-dev/JuMP.jl/pull/3776

I'm marking this as a bug fix, because we already had value(::Number), just not value(::Function, ::Number).