MultivariatePolynomials.jl icon indicating copy to clipboard operation
MultivariatePolynomials.jl copied to clipboard

`==(α, q::MP.RationalPoly)` and `==(p::MP.AbstractPolynomialLike, α)` cause many invalidations

Open sumiya11 opened this issue 1 year ago • 5 comments

To check this, I run

# from https://sciml.ai/news/2022/09/21/compile_time/
using SnoopCompile

invalidations = @snoopr begin
    using AbstractAlgebra
    R, (x,y) = polynomial_ring(QQ, ["x","y"], ordering=:degrevlex)
    using Groebner
    groebner([x^2 - y^2, x*y^2 + x])
end

trees = SnoopCompile.invalidation_trees(invalidations);

@show length(SnoopCompile.uinvalidated(invalidations)) # show total invalidations (1102)

show(trees[end-1])
show(trees[end-2])

(Groebner.jl loads MultivariatePolynomials.jl).

I have

 inserting ==(α, q::MultivariatePolynomials.RationalPoly) @ MultivariatePolynomials C:\Users\User\.julia\packages\MultivariatePolynomials\TpRhf\src\comparison.jl:118 invalidated:
   backedges: 1: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Core.MethodInstance, ::Any) (6 children)
              2: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Module, ::Any) (11 children)
              3: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Method, ::Any) (33 children)
              4: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Symbol, ::Any) (154 children)
              5: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Core.TypeName, ::Any) (164 children)
              6: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Symbol, ::Any) (1 children)
              7: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Symbol, ::Any) (4 children)

 inserting ==(p::MultivariatePolynomials.AbstractPolynomialLike, α) @ MultivariatePolynomials C:\Users\User\.julia\packages\MultivariatePolynomials\TpRhf\src\operators.jl:65 invalidated:
   backedges: 1: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::Cthulhu.DInfo.DebugInfo) (6 children)
              2: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::Core.MethodInstance) (8 children)
              3: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::Symbol) (420 children)
              4: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::typeof(display)) (2 children)
              5: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::Symbol) (1 children)
              6: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::Symbol) (1 children)
              7: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::typeof(display)) (1 children)
              8: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::typeof(display)) (11 children)
              9: superseding ==(x, y) @ Base Base.jl:165 with MethodInstance for ==(::Any, ::Symbol) (2 children)

This amounts to 825 method invalidations out of 1102.

I wonder if it would be okay to rewrite these methods with the use of isa (or even remove them) ?

sumiya11 avatar Feb 14 '24 18:02 sumiya11

I am not familiar enough with this package to answer that, but if AbstractPolynomialLike and RationalPoly were to subtype Number, then by default they would hit the ==(x::Number, y::Number) = ==(promote(x, y)...) fallback so they could use promote to avoid having to redefine == for Any arguments.

serenity4 avatar Apr 02 '25 13:04 serenity4

What about just changing Any to Union{Number, AbstractPolynomialLike, RationalPoly}?

StefanKarpinski avatar Apr 02 '25 13:04 StefanKarpinski

AbstractPolynomialLike does not subtype Number for the same reason JuMP.AbstractJuMPScalar does not : because it does not satisfy many of the assumptions that are usually satisfied by Number, see https://github.com/jump-dev/MutableArithmetics.jl/issues/47 We aim to support as coefficient Any an not Number because we want to support coefficients that are AbstractArray or JuMP.AbstractJuMPScalar as well. The issue of ==(a, b) = ==(promote(a, b)...) is that this promotion would allocate while for JuMP expressions and polynomials, if you are comparing with an object of a simpler structure, it's cheaper to check if the more complex structure is equal to the simpler one than allocating the more complex one for the simpler object. @sumiya11 what is your suggestion using isa ?

blegat avatar Apr 02 '25 16:04 blegat

If this is causing a lot of issues, I can change Any to Union{Number,MutableArithmetics.AbstractMutable,AbstractArray} which I think would cover most use cases.

blegat avatar Apr 03 '25 11:04 blegat

@sumiya11 what is your suggestion using isa ?

Hum... I am afraid I don't recall what I had in mind back then.

sumiya11 avatar Apr 05 '25 18:04 sumiya11