diff --git a/src/Utilities/functions.jl b/src/Utilities/functions.jl index 0d78c7089e..3ae1e49d06 100644 --- a/src/Utilities/functions.jl +++ b/src/Utilities/functions.jl @@ -1003,15 +1003,19 @@ is_canonical(::Union{MOI.VariableIndex,MOI.VectorOfVariables}) = true function is_canonical(f::MOI.ScalarNonlinearFunction) # Don't use recursion here. This gets called for all scalar nonlinear # constraints. - stack = Any[arg for arg in f.args] + # A subexpression may appear several times in the expression graph, we + # visit it only once. + visited = Base.IdSet{MOI.ScalarNonlinearFunction}() + stack = MOI.ScalarNonlinearFunction[f] while !isempty(stack) - arg = pop!(stack) - if arg isa MOI.ScalarNonlinearFunction - for a in arg.args - push!(stack, a) - end - else - if !is_canonical(arg) + g = pop!(stack) + for arg in g.args + if arg isa MOI.ScalarNonlinearFunction + if !(arg in visited) + push!(visited, arg) + push!(stack, arg) + end + elseif !is_canonical(arg) return false end end diff --git a/test/Utilities/test_functions.jl b/test/Utilities/test_functions.jl index b49427bf65..a53e5c8b23 100644 --- a/test/Utilities/test_functions.jl +++ b/test/Utilities/test_functions.jl @@ -2101,6 +2101,23 @@ function test_ScalarNonlinearFunction_is_canonical() return end +function test_is_canonical_ScalarNonlinearFunction_common_subexpressions() + x = MOI.VariableIndex(1) + # The tree has 2^N leaves, the expression graph has N nodes + N = 100 + f = MOI.ScalarNonlinearFunction(:log, Any[1.0*x]) + for _ in 1:N + f = MOI.ScalarNonlinearFunction(:+, Any[f, f]) + end + @test MOI.Utilities.is_canonical(f) + g = MOI.ScalarNonlinearFunction(:log, Any[1.0*x+1.0*x]) + for _ in 1:N + g = MOI.ScalarNonlinearFunction(:+, Any[g, g]) + end + @test !MOI.Utilities.is_canonical(g) + return +end + function test_vector_type() for T in (Int, Float64) @test MOI.Utilities.vector_type(T) == Vector{T}