jump-dev / JuMP.jl

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

Inconsistance between @NLobjective and @objective #2448

Closed sylvaticus closed 3 years ago

sylvaticus commented 3 years ago

Let's consider the following model:

model = Model(with_optimizer(Ipopt.Optimizer))
@variable(model, x >= 0)
@variable(model, y >= 1)

Then the macro @objective can be used both as:

@objective(model, Min, x+2*y)

or

@objective model Min begin
    x+2*y
end

The macro @NLobjective however can be used only as:

@NLobjective(model, Min, exp(x)+y)

But NOT as:

@NLobjective model Min begin
    exp(x)+y
end

The above script would indeed lead to weird error messages (in the case above "exp is not defined for type AbstractVariableRef. Are you trying to build a nonlinear problem? Make sure you use @NLconstraint/@NLobjective.", but it can be more weird when you try to use custom functions).

The best solution for me would be to uniform the two APIs, but at least a better error message could be provided...

Xcross: https://discourse.julialang.org/t/inconsistance-between-nlobjective-and-objective/54520

odow commented 3 years ago
julia> @NLobjective model Min begin
       exp(x) + y
       end
ERROR: exp is not defined for type AbstractVariableRef. Are you trying to build a nonlinear problem? Make sure you use @NLconstraint/@NLobjective.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] exp(::VariableRef) at /Users/oscar/.julia/packages/JuMP/y5vgk/src/operators.jl:283
 [3] top-level scope at REPL[11]:2
 [4] top-level scope at /Users/oscar/.julia/packages/JuMP/y5vgk/src/parse_nlp.jl:255
 [5] top-level scope at /Users/oscar/.julia/packages/JuMP/y5vgk/src/macros.jl:1466
 [6] top-level scope at REPL[11]:1

because

julia> @macroexpand @NLobjective(model, Min, begin
       exp(x) + y
       end)
quote
    #= REPL[13]:1 =#
    JuMP._valid_model(model, :model)
    begin
        #= /Users/oscar/.julia/packages/JuMP/y5vgk/src/macros.jl:1466 =#
        var"#10###259" = begin
                #= /Users/oscar/.julia/packages/JuMP/y5vgk/src/parse_nlp.jl:253 =#
                var"#11#tape" = JuMP.NodeData[]
                #= /Users/oscar/.julia/packages/JuMP/y5vgk/src/parse_nlp.jl:254 =#
                var"#12#values" = JuMP.Float64[]
                #= /Users/oscar/.julia/packages/JuMP/y5vgk/src/parse_nlp.jl:255 =#
                JuMP._parse_NL_expr_runtime(model, begin
                        #= REPL[13]:2 =#
                        exp(x) + y
                    end, var"#11#tape", -1, var"#12#values")
                #= /Users/oscar/.julia/packages/JuMP/y5vgk/src/parse_nlp.jl:256 =#
                JuMP._NonlinearExprData(var"#11#tape", var"#12#values")
            end
        #= /Users/oscar/.julia/packages/JuMP/y5vgk/src/macros.jl:1467 =#
        JuMP.set_objective(model, JuMP._throw_error_for_invalid_sense(JuMP.var"#_error#99"{LineNumberNode,Symbol,Symbol,Expr}(:(#= REPL[13]:1 =#), :model, :Min, quote
    #= REPL[13]:2 =#
    exp(x) + y
end), MathOptInterface.MIN_SENSE), var"#10###259")
    end
end

The fix would appear to be adding an if to list the expression out of the block here: https://github.com/jump-dev/JuMP.jl/blob/2892b68cdd15b1632f9f9f6d5ab396484754e40b/src/parse_nlp.jl#L21-L22