From 1517124527d144f2b32ac656a1393d28693d3720 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Fri, 22 Nov 2024 14:36:07 +1300 Subject: [PATCH 1/6] Fix StackOverflow error in value of GenericNonlinearExpr --- src/nlp_expr.jl | 74 +++++++++++++++++++++++++------------------ test/test_nlp_expr.jl | 11 +++++++ 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index 6e99783c501..1c5c93c6c87 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -673,50 +673,64 @@ function _evaluate_expr( return convert(Float64, expr) end -function _evaluate_user_defined_function( - registry, - f, +struct _ConcreteExpression{T} + head::Symbol + args::Vector{T} +end + +function _evaluate_expr( + registry::MOI.Nonlinear.OperatorRegistry, + f::Function, expr::GenericNonlinearExpr, ) - model = owner_model(expr) - op, nargs = expr.head, length(expr.args) - udf = MOI.get(model, MOI.UserDefinedFunction(op, nargs)) - if udf === nothing - return error( - "Unable to evaluate nonlinear operator $op because it was " * - "not added as an operator.", - ) + ret = _ConcreteExpression{Float64}(expr.head, Float64[]) + stack = Any[] + for arg in reverse(expr.args) + push!(stack, (ret, arg)) + end + while !isempty(stack) + parent, arg = pop!(stack) + if arg isa MOI.ScalarNonlinearFunction + new_ret = _ConcreteExpression{Float64}(arg.head, Float64[]) + push!(parent.args, new_ret) + for child in reverse(arg.args) + push!(stack, (new_ret, child)) + end + else + push!(parent.args, _evaluate_expr(registry, f, arg)) + end end - args = [_evaluate_expr(registry, f, arg) for arg in expr.args] - return first(udf)(args...) + return _evaluate_expr(registry, f, ret) end function _evaluate_expr( registry::MOI.Nonlinear.OperatorRegistry, - f::Function, - expr::GenericNonlinearExpr, + ::Function, + expr::_ConcreteExpression, ) - op = expr.head + op, args, nargs = expr.head, expr.args, length(expr.args) # TODO(odow): uses private function - if !MOI.Nonlinear._is_registered(registry, op, length(expr.args)) - return _evaluate_user_defined_function(registry, f, expr) - end - if length(expr.args) == 1 && haskey(registry.univariate_operator_to_id, op) - arg = _evaluate_expr(registry, f, expr.args[1]) - return MOI.Nonlinear.eval_univariate_function(registry, op, arg) + if !MOI.Nonlinear._is_registered(registry, op, nargs) + model = owner_model(expr) + udf = MOI.get(model, MOI.UserDefinedFunction(op, nargs)) + if udf === nothing + return error( + "Unable to evaluate nonlinear operator $op because it was " * + "not added as an operator.", + ) + end + return first(udf)(args...) + elseif nargs == 1 && haskey(registry.univariate_operator_to_id, op) + return MOI.Nonlinear.eval_univariate_function(registry, op, args[1]) elseif haskey(registry.multivariate_operator_to_id, op) - args = [_evaluate_expr(registry, f, arg) for arg in expr.args] return MOI.Nonlinear.eval_multivariate_function(registry, op, args) elseif haskey(registry.logic_operator_to_id, op) - @assert length(expr.args) == 2 - x = _evaluate_expr(registry, f, expr.args[1]) - y = _evaluate_expr(registry, f, expr.args[2]) - return MOI.Nonlinear.eval_logic_function(registry, op, x, y) + @assert nargs == 2 + return MOI.Nonlinear.eval_logic_function(registry, op, args[1], args[2]) else @assert haskey(registry.comparison_operator_to_id, op) - @assert length(expr.args) == 2 - x = _evaluate_expr(registry, f, expr.args[1]) - y = _evaluate_expr(registry, f, expr.args[2]) + @assert nargs == 2 + x, y = args return MOI.Nonlinear.eval_comparison_function(registry, op, x, y) end end diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 8bb131dfa4f..28dd26f8f0e 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -371,6 +371,17 @@ function test_extension_recursion_stackoverflow( return end +function test_evaluate_expr_stackoverflow() + N = 10_000 + model = Model() + @variable(model, x[1:N], start = 0) + f = prod(sum(x[i-1:i]) for i in 2:N) + @test value(start_value, f) == 0.0 + set_start_value.(x, 1.0) + @test value(start_value, f) == Inf + return +end + function test_nlobjective_with_nlexpr() model = Model() @variable(model, x) From 2796ec829e2fad005dfd73183382c742763293f5 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Fri, 22 Nov 2024 16:41:59 +1300 Subject: [PATCH 2/6] Update --- src/nlp_expr.jl | 9 +++++---- test/test_nlp_expr.jl | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index 1c5c93c6c87..bd4b354d952 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -673,9 +673,10 @@ function _evaluate_expr( return convert(Float64, expr) end -struct _ConcreteExpression{T} +struct _ConcreteExpression head::Symbol - args::Vector{T} + args::Vector{Real} + _ConcreteExpression(head::Symbol) = new(head, Real[]) end function _evaluate_expr( @@ -683,7 +684,7 @@ function _evaluate_expr( f::Function, expr::GenericNonlinearExpr, ) - ret = _ConcreteExpression{Float64}(expr.head, Float64[]) + ret = _ConcreteExpression(expr.head) stack = Any[] for arg in reverse(expr.args) push!(stack, (ret, arg)) @@ -691,7 +692,7 @@ function _evaluate_expr( while !isempty(stack) parent, arg = pop!(stack) if arg isa MOI.ScalarNonlinearFunction - new_ret = _ConcreteExpression{Float64}(arg.head, Float64[]) + new_ret = _ConcreteExpression(arg.head) push!(parent.args, new_ret) for child in reverse(arg.args) push!(stack, (new_ret, child)) diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 28dd26f8f0e..64477eda9fb 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -382,6 +382,22 @@ function test_evaluate_expr_stackoverflow() return end +function test_evaluate_expr_stackoverflow_user_defined_function() + N = 10_000 + f(x, y) = *(x, y) + model = Model() + @variable(model, x[1:N], start = 0) + @operator(model, op_f, 2, f) + y = x[1] + for i in 2:N + y = op_f(x[i], y) + end + @test value(start_value, y) == 0.0 + set_start_value.(x, 1.0) + @test value(start_value, y) == 1.0 + return +end + function test_nlobjective_with_nlexpr() model = Model() @variable(model, x) From 2bf5bf8fb33a3bdd2f780af439246fe8e3070862 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Fri, 22 Nov 2024 16:43:39 +1300 Subject: [PATCH 3/6] Update --- src/nlp_expr.jl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index bd4b354d952..601249d5ef3 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -693,7 +693,7 @@ function _evaluate_expr( parent, arg = pop!(stack) if arg isa MOI.ScalarNonlinearFunction new_ret = _ConcreteExpression(arg.head) - push!(parent.args, new_ret) + push!(parent.args, (arg, new_ret)) for child in reverse(arg.args) push!(stack, (new_ret, child)) end @@ -701,18 +701,19 @@ function _evaluate_expr( push!(parent.args, _evaluate_expr(registry, f, arg)) end end - return _evaluate_expr(registry, f, ret) + return _evaluate_expr(registry, f, (expr, ret)) end function _evaluate_expr( registry::MOI.Nonlinear.OperatorRegistry, ::Function, - expr::_ConcreteExpression, + input::Tuple{<:GenericNonlinearExpr,_ConcreteExpression}, ) + f, expr = input op, args, nargs = expr.head, expr.args, length(expr.args) # TODO(odow): uses private function if !MOI.Nonlinear._is_registered(registry, op, nargs) - model = owner_model(expr) + model = owner_model(f) udf = MOI.get(model, MOI.UserDefinedFunction(op, nargs)) if udf === nothing return error( From af46406804a4475d2e22991d2e8f0122da95b524 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Sat, 23 Nov 2024 16:40:05 +1300 Subject: [PATCH 4/6] Update --- src/nlp_expr.jl | 38 ++++++++++++++++---------------------- test/test_nlp_expr.jl | 2 +- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index 601249d5ef3..8238b95fc43 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -673,47 +673,41 @@ function _evaluate_expr( return convert(Float64, expr) end -struct _ConcreteExpression - head::Symbol - args::Vector{Real} - _ConcreteExpression(head::Symbol) = new(head, Real[]) -end - function _evaluate_expr( registry::MOI.Nonlinear.OperatorRegistry, f::Function, expr::GenericNonlinearExpr, ) - ret = _ConcreteExpression(expr.head) - stack = Any[] - for arg in reverse(expr.args) - push!(stack, (ret, arg)) - end + stack, result_stack = Any[expr], Real[] while !isempty(stack) - parent, arg = pop!(stack) - if arg isa MOI.ScalarNonlinearFunction - new_ret = _ConcreteExpression(arg.head) - push!(parent.args, (arg, new_ret)) + arg = pop!(stack) + if arg isa GenericNonlinearExpr + # wrap arg in a Tuple to catch when we should evaluate. + push!(stack, (arg,)) for child in reverse(arg.args) - push!(stack, (new_ret, child)) + push!(stack, child) end + elseif arg isa Tuple{<:GenericNonlinearExpr} + f_expr = only(arg) + args = reverse([pop!(result_stack) for _ in 1:length(f_expr.args)]) + push!(result_stack, _evaluate_expr(registry, f, f_expr, args)) else - push!(parent.args, _evaluate_expr(registry, f, arg)) + push!(result_stack, _evaluate_expr(registry, f, arg)) end end - return _evaluate_expr(registry, f, (expr, ret)) + return only(result_stack) end function _evaluate_expr( registry::MOI.Nonlinear.OperatorRegistry, ::Function, - input::Tuple{<:GenericNonlinearExpr,_ConcreteExpression}, + f_expr::GenericNonlinearExpr, + args::Vector ) - f, expr = input - op, args, nargs = expr.head, expr.args, length(expr.args) + op, nargs = f_expr.head, length(args) # TODO(odow): uses private function if !MOI.Nonlinear._is_registered(registry, op, nargs) - model = owner_model(f) + model = owner_model(f_expr) udf = MOI.get(model, MOI.UserDefinedFunction(op, nargs)) if udf === nothing return error( diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 64477eda9fb..bfcbfbf8829 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -375,7 +375,7 @@ function test_evaluate_expr_stackoverflow() N = 10_000 model = Model() @variable(model, x[1:N], start = 0) - f = prod(sum(x[i-1:i]) for i in 2:N) + f = prod(sum(x[1:i]) for i in 1:N) @test value(start_value, f) == 0.0 set_start_value.(x, 1.0) @test value(start_value, f) == Inf From 6907bd514fc051c150bca369ea551264df30ff03 Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Sat, 23 Nov 2024 16:43:55 +1300 Subject: [PATCH 5/6] Fix formatting --- src/nlp_expr.jl | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index 8238b95fc43..adf6fd59ef9 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -678,18 +678,19 @@ function _evaluate_expr( f::Function, expr::GenericNonlinearExpr, ) + # The result_stack needs to be ::Real because operators like || return a + # ::Bool. Also, some inputs may be ::Int. stack, result_stack = Any[expr], Real[] while !isempty(stack) arg = pop!(stack) if arg isa GenericNonlinearExpr - # wrap arg in a Tuple to catch when we should evaluate. - push!(stack, (arg,)) - for child in reverse(arg.args) + push!(stack, (arg,)) # wrap in (,) to catch when we should eval it. + for child in arg.args push!(stack, child) end elseif arg isa Tuple{<:GenericNonlinearExpr} f_expr = only(arg) - args = reverse([pop!(result_stack) for _ in 1:length(f_expr.args)]) + args = Real[pop!(result_stack) for _ in 1:length(f_expr.args)] push!(result_stack, _evaluate_expr(registry, f, f_expr, args)) else push!(result_stack, _evaluate_expr(registry, f, arg)) @@ -702,7 +703,7 @@ function _evaluate_expr( registry::MOI.Nonlinear.OperatorRegistry, ::Function, f_expr::GenericNonlinearExpr, - args::Vector + args::Vector, ) op, nargs = f_expr.head, length(args) # TODO(odow): uses private function From 47cfa8ebb1f6adbe88184049a884077fbbd2fb0b Mon Sep 17 00:00:00 2001 From: Oscar Dowson Date: Sat, 23 Nov 2024 17:17:12 +1300 Subject: [PATCH 6/6] Update --- src/nlp_expr.jl | 66 +++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index adf6fd59ef9..dda46d90350 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -690,8 +690,37 @@ function _evaluate_expr( end elseif arg isa Tuple{<:GenericNonlinearExpr} f_expr = only(arg) - args = Real[pop!(result_stack) for _ in 1:length(f_expr.args)] - push!(result_stack, _evaluate_expr(registry, f, f_expr, args)) + op, nargs = f_expr.head, length(f_expr.args) + # TODO(odow): uses private function + result = if !MOI.Nonlinear._is_registered(registry, op, nargs) + model = owner_model(f_expr) + udf = MOI.get(model, MOI.UserDefinedFunction(op, nargs)) + if udf === nothing + return error( + "Unable to evaluate nonlinear operator $op because " * + "it was not added as an operator.", + ) + end + first(udf)((pop!(result_stack) for _ in 1:nargs)...) + elseif nargs == 1 && haskey(registry.univariate_operator_to_id, op) + x = pop!(result_stack) + MOI.Nonlinear.eval_univariate_function(registry, op, x) + elseif haskey(registry.multivariate_operator_to_id, op) + args = Real[pop!(result_stack) for _ in 1:nargs] + MOI.Nonlinear.eval_multivariate_function(registry, op, args) + elseif haskey(registry.logic_operator_to_id, op) + @assert nargs == 2 + x = pop!(result_stack) + y = pop!(result_stack) + MOI.Nonlinear.eval_logic_function(registry, op, x, y) + else + @assert haskey(registry.comparison_operator_to_id, op) + @assert nargs == 2 + x = pop!(result_stack) + y = pop!(result_stack) + MOI.Nonlinear.eval_comparison_function(registry, op, x, y) + end + push!(result_stack, result) else push!(result_stack, _evaluate_expr(registry, f, arg)) end @@ -699,39 +728,6 @@ function _evaluate_expr( return only(result_stack) end -function _evaluate_expr( - registry::MOI.Nonlinear.OperatorRegistry, - ::Function, - f_expr::GenericNonlinearExpr, - args::Vector, -) - op, nargs = f_expr.head, length(args) - # TODO(odow): uses private function - if !MOI.Nonlinear._is_registered(registry, op, nargs) - model = owner_model(f_expr) - udf = MOI.get(model, MOI.UserDefinedFunction(op, nargs)) - if udf === nothing - return error( - "Unable to evaluate nonlinear operator $op because it was " * - "not added as an operator.", - ) - end - return first(udf)(args...) - elseif nargs == 1 && haskey(registry.univariate_operator_to_id, op) - return MOI.Nonlinear.eval_univariate_function(registry, op, args[1]) - elseif haskey(registry.multivariate_operator_to_id, op) - return MOI.Nonlinear.eval_multivariate_function(registry, op, args) - elseif haskey(registry.logic_operator_to_id, op) - @assert nargs == 2 - return MOI.Nonlinear.eval_logic_function(registry, op, args[1], args[2]) - else - @assert haskey(registry.comparison_operator_to_id, op) - @assert nargs == 2 - x, y = args - return MOI.Nonlinear.eval_comparison_function(registry, op, x, y) - end -end - # MutableArithmetics.jl and promotion function Base.promote_rule(