From e8265906a4be223442e068f14645b802c5f1b5bf Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 13 Sep 2024 12:57:05 +1200 Subject: [PATCH 1/2] Fix an illegal simplification in MA.operate!! for NonlinearExpr --- src/nlp_expr.jl | 11 ++++++++--- test/test_nlp_expr.jl | 9 +++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/nlp_expr.jl b/src/nlp_expr.jl index 6393ea297e0..066610bb8ba 100644 --- a/src/nlp_expr.jl +++ b/src/nlp_expr.jl @@ -408,10 +408,15 @@ function _MA.operate!!( _throw_if_not_real(x) if any(isequal(_MA.Zero()), args) return x - elseif x.head == :+ - push!(x.args, *(args...)) - return x end + # It may seem like we should do this performance optimization, but it is NOT + # safe. See JuMP#3825. The issue is that even though we're calling operate!! + # `x` is not mutable, because it may, amoungst other things, be aliased in + # one of the args + # elseif x.head == :+ + # push!(x.args, *(args...)) + # return x + # end return +(x, *(args...)) end diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index a92d624462e..3ef010f6e26 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -873,6 +873,15 @@ function test_ma_zero_in_operate!!() return end +function test_ma_operate!!_nested_sum() + model = Model() + @variable(model, x) + y = NonlinearExpr(:+, Any[x]) + z = MA.operate!!(MA.add_mul, y, y) + @test isequal_canonical(z, @force_nonlinear(+(+x, +x))) + return +end + function test_nonlinear_operator_inferred() model = Model() @variable(model, x) From b2fbd671a588d70f6832ec548e5102b4a315154f Mon Sep 17 00:00:00 2001 From: odow Date: Fri, 13 Sep 2024 12:58:53 +1200 Subject: [PATCH 2/2] Update --- test/test_nlp_expr.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_nlp_expr.jl b/test/test_nlp_expr.jl index 3ef010f6e26..afc555f7da4 100644 --- a/test/test_nlp_expr.jl +++ b/test/test_nlp_expr.jl @@ -878,6 +878,7 @@ function test_ma_operate!!_nested_sum() @variable(model, x) y = NonlinearExpr(:+, Any[x]) z = MA.operate!!(MA.add_mul, y, y) + @test isequal_canonical(y, @force_nonlinear(+x)) @test isequal_canonical(z, @force_nonlinear(+(+x, +x))) return end