MultivariatePolynomials.jl
MultivariatePolynomials.jl copied to clipboard
`==(α, q::MP.RationalPoly)` and `==(p::MP.AbstractPolynomialLike, α)` cause many invalidations
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) ?
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.
What about just changing Any to Union{Number, AbstractPolynomialLike, RationalPoly}?
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 ?
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.
@sumiya11 what is your suggestion using isa ?
Hum... I am afraid I don't recall what I had in mind back then.