From c4ba5895198f4c326f16984a2e40ca8a6cd18cfc Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 2 May 2024 07:53:50 +1200 Subject: [PATCH 1/6] Fix error message if NonlinearExpression is mixed with new NL API --- src/nlp_expr.jl | 18 ++++++++++++++++++ test/test_nlp_expr.jl | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index bbbdf6c11ee..342916a7f35 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -87,6 +87,7 @@ struct GenericNonlinearExpr{V<:AbstractVariableRef} <: AbstractJuMPScalar ) where {V<:AbstractVariableRef} for arg in args _throw_if_not_real(arg) + _throw_if_legacy(arg) end return new{V}(head, Any[a for a in args]) end @@ -97,11 +98,28 @@ struct GenericNonlinearExpr{V<:AbstractVariableRef} <: AbstractJuMPScalar ) where {V<:AbstractVariableRef} for arg in args _throw_if_not_real(arg) + _throw_if_legacy(arg) end return new{V}(head, args) end end +_throw_if_legacy(::Any) = nothing + +function _throw_if_legacy(arg::Union{NonlinearExpression,NonlinearParameter}) + return error( + "Cannot create a nonlinear expression that mixes features from " * + "both the legacy (macros beginning with `@NL`) and new " * + "(`NonlinearExpr`) nonlinear interfaces. You must use one or " * + "the other. Got: $arg", + ) +end + +# This method is necessary to catch errors in an expression like p * x where +# p is a nonlinear parameter. +variable_ref_type(::NonlinearParameter) = GenericVariableRef{Float64} +variable_ref_type(::NonlinearExpression) = GenericVariableRef{Float64} + variable_ref_type(::Type{GenericNonlinearExpr}, ::Any) = nothing function variable_ref_type(::Type{GenericNonlinearExpr}, x::AbstractJuMPScalar) diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 23ba19c781e..6311b192216 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -1092,4 +1092,40 @@ function test_error_nonlinear_expr_complex_constructor() return end +function test_error_legacy_expression_constructor() + model = Model() + @variable(model, x) + @NLexpression(model, arg, x^3) + err = ErrorException( + "Cannot create a nonlinear expression that mixes features from " * + "both the legacy (macros beginning with `@NL`) and new " * + "(`NonlinearExpr`) nonlinear interfaces. You must use one or " * + "the other. Got: $arg", + ) + @test_throws err GenericNonlinearExpr(:*, Any[x, arg]) + @test_throws err GenericNonlinearExpr(:*, x, arg) + @test_throws err (x * arg) + @test_throws err (arg * x) + return +end + +function test_error_legacy_parameter_constructor() + model = Model() + @variable(model, x) + @NLparameter(model, p == 1) + err = ErrorException( + "Cannot create a nonlinear expression that mixes features from " * + "both the legacy (macros beginning with `@NL`) and new " * + "(`NonlinearExpr`) nonlinear interfaces. You must use one or " * + "the other. Got: $p", + ) + @test_throws err GenericNonlinearExpr(:*, Any[x, p]) + @test_throws err GenericNonlinearExpr(:*, Any[p, x]) + @test_throws err GenericNonlinearExpr(:*, x, p) + @test_throws err GenericNonlinearExpr(:*, p, x) + @test_throws err (x * p) + @test_throws err (p * x) + return +end + end # module From b76a8b218e490f699fc6d6b08e9d8d581d27e58a Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 2 May 2024 10:05:35 +1200 Subject: [PATCH 2/6] Update --- test/test_nlp_expr.jl | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 6311b192216..58266ae659f 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -371,26 +371,6 @@ function test_extension_recursion_stackoverflow( return end -function test_nlparameter_interaction() - model = Model() - @variable(model, x) - @NLparameter(model, p == 1) - e = x + p - @test e isa GenericNonlinearExpr - @test string(e) == "x + ($p)" - return -end - -function test_nlexpression_interaction() - model = Model() - @variable(model, x) - @NLexpression(model, expr, sin(x)) - e = x + expr - @test e isa GenericNonlinearExpr - @test string(e) == "x + ($expr)" - return -end - function test_nlobjective_with_nlexpr() model = Model() @variable(model, x) @@ -414,18 +394,6 @@ function test_nlconstraint_with_nlexpr() return end -function test_jump_function_nonlinearexpr() - model = Model() - @variable(model, x) - @NLparameter(model, p == 1) - @NLexpression(model, expr1, sin(p + x)) - @NLexpression(model, expr2, sin(expr1)) - nlp = nonlinear_model(model) - @test string(jump_function(model, nlp[index(expr1)])) == "sin(($p) + $x)" - @test string(jump_function(model, nlp[index(expr2)])) == "sin($expr1)" - return -end - function test_constraint_object() model = Model() @variable(model, x) From 04ff92a72e0feab211ae3147ad1bdaa512712595 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 2 May 2024 14:09:30 +1200 Subject: [PATCH 3/6] Update --- src/nlp.jl | 8 ++++++ src/nlp_expr.jl | 13 +++------ test/test_nlp_expr.jl | 62 ++++++++++++++++++++++++++++++------------- 3 files changed, 56 insertions(+), 27 deletions(-) diff --git a/src/nlp.jl b/src/nlp.jl index 53e72f23438..0d82ff476ef 100644 --- a/src/nlp.jl +++ b/src/nlp.jl @@ -209,6 +209,10 @@ struct NonlinearParameter <: AbstractJuMPScalar index::Int end +function check_belongs_to_model(arg::NonlinearParameter, model::AbstractModel) + return arg.model === model +end + function MOI.Nonlinear.parse_expression( model::MOI.Nonlinear.Model, expr::MOI.Nonlinear.Expression, @@ -313,6 +317,10 @@ struct NonlinearExpression <: AbstractJuMPScalar index::Int end +function check_belongs_to_model(arg::NonlinearExpression, model::AbstractModel) + return arg.model === model +end + function MOI.Nonlinear.parse_expression( model::MOI.Nonlinear.Model, expr::MOI.Nonlinear.Expression, diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index 342916a7f35..57a1634c207 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -87,7 +87,6 @@ struct GenericNonlinearExpr{V<:AbstractVariableRef} <: AbstractJuMPScalar ) where {V<:AbstractVariableRef} for arg in args _throw_if_not_real(arg) - _throw_if_legacy(arg) end return new{V}(head, Any[a for a in args]) end @@ -98,20 +97,16 @@ struct GenericNonlinearExpr{V<:AbstractVariableRef} <: AbstractJuMPScalar ) where {V<:AbstractVariableRef} for arg in args _throw_if_not_real(arg) - _throw_if_legacy(arg) end return new{V}(head, args) end end -_throw_if_legacy(::Any) = nothing - -function _throw_if_legacy(arg::Union{NonlinearExpression,NonlinearParameter}) +function moi_function(arg::Union{NonlinearExpression,NonlinearParameter}) return error( - "Cannot create a nonlinear expression that mixes features from " * - "both the legacy (macros beginning with `@NL`) and new " * - "(`NonlinearExpr`) nonlinear interfaces. You must use one or " * - "the other. Got: $arg", + "You cannot mix legacy nonlinear object of type $(typeof(arg)) with " * + "the new nonlinear API. To use the legacy nonlinear API, all " * + "nonlinear objects must be in a `@NL` macro.", ) end diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 58266ae659f..3d6e2b3b748 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -371,6 +371,26 @@ function test_extension_recursion_stackoverflow( return end +function test_nlparameter_interaction() + model = Model() + @variable(model, x) + @NLparameter(model, p == 1) + e = x + p + @test e isa GenericNonlinearExpr + @test string(e) == "x + ($p)" + return +end + +function test_nlexpression_interaction() + model = Model() + @variable(model, x) + @NLexpression(model, expr, sin(x)) + e = x + expr + @test e isa GenericNonlinearExpr + @test string(e) == "x + ($expr)" + return +end + function test_nlobjective_with_nlexpr() model = Model() @variable(model, x) @@ -394,6 +414,18 @@ function test_nlconstraint_with_nlexpr() return end +function test_jump_function_nonlinearexpr() + model = Model() + @variable(model, x) + @NLparameter(model, p == 1) + @NLexpression(model, expr1, sin(p + x)) + @NLexpression(model, expr2, sin(expr1)) + nlp = nonlinear_model(model) + @test string(jump_function(model, nlp[index(expr1)])) == "sin(($p) + $x)" + @test string(jump_function(model, nlp[index(expr2)])) == "sin($expr1)" + return +end + function test_constraint_object() model = Model() @variable(model, x) @@ -1065,15 +1097,13 @@ function test_error_legacy_expression_constructor() @variable(model, x) @NLexpression(model, arg, x^3) err = ErrorException( - "Cannot create a nonlinear expression that mixes features from " * - "both the legacy (macros beginning with `@NL`) and new " * - "(`NonlinearExpr`) nonlinear interfaces. You must use one or " * - "the other. Got: $arg", + "You cannot mix legacy nonlinear object of type $(typeof(arg)) with " * + "the new nonlinear API. To use the legacy nonlinear API, all " * + "nonlinear objects must be in a `@NL` macro.", ) - @test_throws err GenericNonlinearExpr(:*, Any[x, arg]) - @test_throws err GenericNonlinearExpr(:*, x, arg) - @test_throws err (x * arg) - @test_throws err (arg * x) + @test_throws err @objective(model, Min, arg) + @test_throws err @constraint(model, arg <= 0) + @test_throws err @constraint(model, arg in MOI.LessThan(0.0)) return end @@ -1082,17 +1112,13 @@ function test_error_legacy_parameter_constructor() @variable(model, x) @NLparameter(model, p == 1) err = ErrorException( - "Cannot create a nonlinear expression that mixes features from " * - "both the legacy (macros beginning with `@NL`) and new " * - "(`NonlinearExpr`) nonlinear interfaces. You must use one or " * - "the other. Got: $p", + "You cannot mix legacy nonlinear object of type $(typeof(p)) with " * + "the new nonlinear API. To use the legacy nonlinear API, all " * + "nonlinear objects must be in a `@NL` macro.", ) - @test_throws err GenericNonlinearExpr(:*, Any[x, p]) - @test_throws err GenericNonlinearExpr(:*, Any[p, x]) - @test_throws err GenericNonlinearExpr(:*, x, p) - @test_throws err GenericNonlinearExpr(:*, p, x) - @test_throws err (x * p) - @test_throws err (p * x) + @test_throws err @objective(model, Min, p) + @test_throws err @constraint(model, p <= 0) + @test_throws err @constraint(model, p in MOI.LessThan(0.0)) return end From 74f265bb042c19c5f6ab91bc9bb1c11398255805 Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 2 May 2024 14:16:43 +1200 Subject: [PATCH 4/6] Update --- test/test_nlp_expr.jl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 3d6e2b3b748..84f1707cbb7 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -1104,6 +1104,8 @@ function test_error_legacy_expression_constructor() @test_throws err @objective(model, Min, arg) @test_throws err @constraint(model, arg <= 0) @test_throws err @constraint(model, arg in MOI.LessThan(0.0)) + @test_throws err moi_function(arg * x) + @test_throws err moi_function(x * arg) return end @@ -1119,6 +1121,8 @@ function test_error_legacy_parameter_constructor() @test_throws err @objective(model, Min, p) @test_throws err @constraint(model, p <= 0) @test_throws err @constraint(model, p in MOI.LessThan(0.0)) + @test_throws err moi_function(p * x) + @test_throws err moi_function(x * p) return end From fbe96e521bc5ecaacb1e59f57f97df34626f2d4b Mon Sep 17 00:00:00 2001 From: odow Date: Thu, 2 May 2024 14:22:28 +1200 Subject: [PATCH 5/6] Update --- src/nlp.jl | 35 +++++++++++++++++++++++++++++++++++ src/nlp_expr.jl | 13 ------------- test/test_nlp_expr.jl | 27 +++++++++++++++++++++------ 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/nlp.jl b/src/nlp.jl index 0d82ff476ef..cfbae6a094a 100644 --- a/src/nlp.jl +++ b/src/nlp.jl @@ -213,6 +213,27 @@ function check_belongs_to_model(arg::NonlinearParameter, model::AbstractModel) return arg.model === model end +variable_ref_type(arg::NonlinearParameter) = variable_ref_type(arg.model) + +function moi_function(p::NonlinearParameter) + return error( + """ + Cannot mix a legacy NonlinearParameter with the new nonlinear API. + + Got: $p + + To update, replace calls to: + ```julia + @NLparameter(model, p == 1) + ``` + with + ```julia + @variable(model, p in Parameter(1)) + ``` + """, + ) +end + function MOI.Nonlinear.parse_expression( model::MOI.Nonlinear.Model, expr::MOI.Nonlinear.Expression, @@ -321,6 +342,20 @@ function check_belongs_to_model(arg::NonlinearExpression, model::AbstractModel) return arg.model === model end +variable_ref_type(arg::NonlinearExpression) = variable_ref_type(arg.model) + +function moi_function(arg::NonlinearExpression) + return error( + """ + Cannot mix a legacy NonlinearExpression with the new nonlinear API. + + Got: $arg + + To update, replace all calls to `@NLexpression` with `@expression`. + """, + ) +end + function MOI.Nonlinear.parse_expression( model::MOI.Nonlinear.Model, expr::MOI.Nonlinear.Expression, diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index 57a1634c207..bbbdf6c11ee 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -102,19 +102,6 @@ struct GenericNonlinearExpr{V<:AbstractVariableRef} <: AbstractJuMPScalar end end -function moi_function(arg::Union{NonlinearExpression,NonlinearParameter}) - return error( - "You cannot mix legacy nonlinear object of type $(typeof(arg)) with " * - "the new nonlinear API. To use the legacy nonlinear API, all " * - "nonlinear objects must be in a `@NL` macro.", - ) -end - -# This method is necessary to catch errors in an expression like p * x where -# p is a nonlinear parameter. -variable_ref_type(::NonlinearParameter) = GenericVariableRef{Float64} -variable_ref_type(::NonlinearExpression) = GenericVariableRef{Float64} - variable_ref_type(::Type{GenericNonlinearExpr}, ::Any) = nothing function variable_ref_type(::Type{GenericNonlinearExpr}, x::AbstractJuMPScalar) diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 84f1707cbb7..02ebe19daba 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -1097,9 +1097,13 @@ function test_error_legacy_expression_constructor() @variable(model, x) @NLexpression(model, arg, x^3) err = ErrorException( - "You cannot mix legacy nonlinear object of type $(typeof(arg)) with " * - "the new nonlinear API. To use the legacy nonlinear API, all " * - "nonlinear objects must be in a `@NL` macro.", + """ + Cannot mix a legacy NonlinearExpression with the new nonlinear API. + + Got: $arg + + To update, replace all calls to `@NLexpression` with `@expression`. + """, ) @test_throws err @objective(model, Min, arg) @test_throws err @constraint(model, arg <= 0) @@ -1114,9 +1118,20 @@ function test_error_legacy_parameter_constructor() @variable(model, x) @NLparameter(model, p == 1) err = ErrorException( - "You cannot mix legacy nonlinear object of type $(typeof(p)) with " * - "the new nonlinear API. To use the legacy nonlinear API, all " * - "nonlinear objects must be in a `@NL` macro.", + """ + Cannot mix a legacy NonlinearParameter with the new nonlinear API. + + Got: $p + + To update, replace calls to: + ```julia + @NLparameter(model, p == 1) + ``` + with + ```julia + @variable(model, p in Parameter(1)) + ``` + """, ) @test_throws err @objective(model, Min, p) @test_throws err @constraint(model, p <= 0) From 854e7d0244d6f38ab7ba3f4544282e75cb83d9ed Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 3 May 2024 10:40:45 +1200 Subject: [PATCH 6/6] Update --- src/nlp.jl | 10 ++++++++-- src/nlp_expr.jl | 2 ++ test/test_nlp_expr.jl | 42 ++++++------------------------------------ 3 files changed, 16 insertions(+), 38 deletions(-) diff --git a/src/nlp.jl b/src/nlp.jl index cfbae6a094a..64f87b75a08 100644 --- a/src/nlp.jl +++ b/src/nlp.jl @@ -3,6 +3,8 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at https://mozilla.org/MPL/2.0/. +_throw_if_legacy_error(::Any) = nothing + """ nonlinear_model( model::GenericModel; @@ -215,7 +217,7 @@ end variable_ref_type(arg::NonlinearParameter) = variable_ref_type(arg.model) -function moi_function(p::NonlinearParameter) +function _throw_if_legacy_error(p::NonlinearParameter) return error( """ Cannot mix a legacy NonlinearParameter with the new nonlinear API. @@ -234,6 +236,8 @@ function moi_function(p::NonlinearParameter) ) end +moi_function(p::NonlinearParameter) = _throw_if_legacy_error(p) + function MOI.Nonlinear.parse_expression( model::MOI.Nonlinear.Model, expr::MOI.Nonlinear.Expression, @@ -344,7 +348,7 @@ end variable_ref_type(arg::NonlinearExpression) = variable_ref_type(arg.model) -function moi_function(arg::NonlinearExpression) +function _throw_if_legacy_error(arg::NonlinearExpression) return error( """ Cannot mix a legacy NonlinearExpression with the new nonlinear API. @@ -356,6 +360,8 @@ function moi_function(arg::NonlinearExpression) ) end +moi_function(arg::NonlinearExpression) = _throw_if_legacy_error(arg) + function MOI.Nonlinear.parse_expression( model::MOI.Nonlinear.Model, expr::MOI.Nonlinear.Expression, diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index bbbdf6c11ee..53e62fc8110 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -87,6 +87,7 @@ struct GenericNonlinearExpr{V<:AbstractVariableRef} <: AbstractJuMPScalar ) where {V<:AbstractVariableRef} for arg in args _throw_if_not_real(arg) + _throw_if_legacy_error(arg) end return new{V}(head, Any[a for a in args]) end @@ -97,6 +98,7 @@ struct GenericNonlinearExpr{V<:AbstractVariableRef} <: AbstractJuMPScalar ) where {V<:AbstractVariableRef} for arg in args _throw_if_not_real(arg) + _throw_if_legacy_error(arg) end return new{V}(head, args) end diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 02ebe19daba..a92d624462e 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -371,26 +371,6 @@ function test_extension_recursion_stackoverflow( return end -function test_nlparameter_interaction() - model = Model() - @variable(model, x) - @NLparameter(model, p == 1) - e = x + p - @test e isa GenericNonlinearExpr - @test string(e) == "x + ($p)" - return -end - -function test_nlexpression_interaction() - model = Model() - @variable(model, x) - @NLexpression(model, expr, sin(x)) - e = x + expr - @test e isa GenericNonlinearExpr - @test string(e) == "x + ($expr)" - return -end - function test_nlobjective_with_nlexpr() model = Model() @variable(model, x) @@ -414,18 +394,6 @@ function test_nlconstraint_with_nlexpr() return end -function test_jump_function_nonlinearexpr() - model = Model() - @variable(model, x) - @NLparameter(model, p == 1) - @NLexpression(model, expr1, sin(p + x)) - @NLexpression(model, expr2, sin(expr1)) - nlp = nonlinear_model(model) - @test string(jump_function(model, nlp[index(expr1)])) == "sin(($p) + $x)" - @test string(jump_function(model, nlp[index(expr2)])) == "sin($expr1)" - return -end - function test_constraint_object() model = Model() @variable(model, x) @@ -1108,8 +1076,9 @@ function test_error_legacy_expression_constructor() @test_throws err @objective(model, Min, arg) @test_throws err @constraint(model, arg <= 0) @test_throws err @constraint(model, arg in MOI.LessThan(0.0)) - @test_throws err moi_function(arg * x) - @test_throws err moi_function(x * arg) + @test_throws err moi_function(arg) + @test_throws err x * arg + @test_throws err arg * x return end @@ -1136,8 +1105,9 @@ function test_error_legacy_parameter_constructor() @test_throws err @objective(model, Min, p) @test_throws err @constraint(model, p <= 0) @test_throws err @constraint(model, p in MOI.LessThan(0.0)) - @test_throws err moi_function(p * x) - @test_throws err moi_function(x * p) + @test_throws err moi_function(p) + @test_throws err x * p + @test_throws err p * x return end