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

Should error when trying to mix `NonlinearExpression` and `GenericNonlinearExpr` #3740

Closed pulsipher closed 1 month ago

pulsipher commented 2 months ago

Due to a hacky patch on the old version of InfiniteOpt that uses the NonlinearExpression, it has come to my attention that the current JuMP API let's users make GenericNonlinearExprs that contain NonlinearExpressions which ultimately culminates in hard to diagnose error:

julia> using JuMP; m = Model(); @variable(m, x);

julia> @NLexpression(m, myexpr, x^3)
subexpression[1]: x ^ 3.0

julia> @expression(m, newexpr, x * myexpr) # produces a NonlinearExpr that contains the NonlinearExpression
x * (subexpression[1]: x ^ 3.0) 

julia> @objective(m, Min, newexpr)
ERROR: MethodError: no method matching check_belongs_to_model(::NonlinearExpression, ::Model)

Closest candidates are:
  check_belongs_to_model(::ConstraintRef, ::AbstractModel)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\constraints.jl:63
  check_belongs_to_model(::ScalarConstraint, ::Any)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\constraints.jl:614
  check_belongs_to_model(::VectorConstraint, ::Any)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\constraints.jl:674
  ...

Stacktrace:
 [1] check_belongs_to_model(expr::NonlinearExpr, model::Model)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\nlp_expr.jl:523
 [2] set_objective_function
   @ ~\.julia\packages\JuMP\as6Ji\src\objective.jl:279 [inlined]
 [3] set_objective(model::Model, sense::MathOptInterface.OptimizationSense, func::NonlinearExpr)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\objective.jl:326
 [4] macro expansion
   @ ~\.julia\packages\JuMP\as6Ji\src\macros\@objective.jl:70 [inlined]
 [5] top-level scope
   @ REPL[64]:1

We get a similar error when trying to to set the objective with the NonlinearExpression directly:

julia> @objective(m, Min, myexpr)
ERROR: MethodError: no method matching check_belongs_to_model(::NonlinearExpression, ::Model)

Closest candidates are:
  check_belongs_to_model(::ConstraintRef, ::AbstractModel)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\constraints.jl:63
  check_belongs_to_model(::ScalarConstraint, ::Any)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\constraints.jl:614
  check_belongs_to_model(::VectorConstraint, ::Any)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\constraints.jl:674
  ...

Stacktrace:
 [1] set_objective_function(model::Model, func::NonlinearExpression)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\objective.jl:279
 [2] set_objective(model::Model, sense::MathOptInterface.OptimizationSense, func::NonlinearExpression)
   @ JuMP ~\.julia\packages\JuMP\as6Ji\src\objective.jl:326
 [3] macro expansion
   @ ~\.julia\packages\JuMP\as6Ji\src\macros\@objective.jl:70 [inlined]
 [4] top-level scope
   @ REPL[65]:1

Also, myexpr (which is the faulty NonlinearExpr) works with @NLobjective:

@NLobjective(m, Min, myexpr)

It seems like more error checking should be added to JuMP to prevent users from mixing the two nonlinear interfaces. In particular, I think we should do the following:

odow commented 2 months ago

We catch things in optimize!. I just didn't account for the case of NonlinearExpression and NonlinearParameter creating things that could appear in other expressions.