Skip to content

Invalidation #339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lrnv opened this issue Nov 2, 2023 · 3 comments · Fixed by #344
Closed

Invalidation #339

lrnv opened this issue Nov 2, 2023 · 3 comments · Fixed by #344

Comments

@lrnv
Copy link

lrnv commented Nov 2, 2023

hey,

using this tutorial on my particular workflow, I endeed up with a lot of invalidations in TaylorSeries.jl as follows:

julia> trees[end]
inserting convert(::Type{Array{TaylorSeries.Taylor1{TaylorSeries.TaylorN{T}}, N}}, s::Array{TaylorSeries.TaylorN{TaylorSeries.Taylor1{T}}, N}) where {T<:Union{Real, Complex}, N} @ TaylorSeries C:\Users\lrnv\.julia\packages\TaylorSeries\OLHVb\src\conversion.jl:140 invalidated:
   backedges: 1: superseding convert(::Type{T}, a::AbstractArray) where T<:Array @ Base array.jl:613 with MethodInstance for convert(::Type{Vector{_A}} where _A, ::Vector) (1 children)
              2: superseding convert(::Type{T}, a::AbstractArray) where T<:Array @ Base array.jl:613 with MethodInstance for convert(::Type{<:Vector}, ::Vector) (263 children)

julia> 

Looks like the problematic method is the convert method there :

function convert(::Type{Array{Taylor1{TaylorN{T}},N}},
s::Array{TaylorN{Taylor1{T}},N}) where {T<:NumberNotSeries,N}
v = Array{Taylor1{TaylorN{T}}}(undef, size(s))
@simd for ind in eachindex(s)
@inbounds v[ind] = convert(Taylor1{TaylorN{T}}, s[ind])
end
return v
end
which invalidates a bunch of convertion already defined in base.

The second one is a promote rule, also from TaylorSeries.jl:

julia> trees[end-1]
inserting promote_rule(::Type{S}, ::Type{T}) where {S<:Union{Real, Complex}, T<:AbstractSeries} @ TaylorSeries C:\Users\lrnv\.julia\packages\TaylorSeries\OLHVb\src\conversion.jl:201 invalidated:
   backedges:  1: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt64}, ::Type{<:Integer}) (1 children)
               2: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt8}, ::Type{<:Integer}) (1 children)       
               3: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt16}, ::Type{<:Integer}) (1 children)      
               4: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{Int64}, ::Type{<:Integer}) (2 children)       
               5: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{Int64}, ::Type{S} where S<:Unsigned) (2 children)
               6: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{Int64}, ::Type{S} where S<:Real) (2 children) 
               7: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt64}, ::Type) (3 children)
               8: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt8}, ::Type) (3 children)
               9: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt16}, ::Type) (3 children)
              10: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt8}, ::Type{T} where T<:Unsigned) (4 children)
              11: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{UInt64}, ::Type{T} where T<:Unsigned) (5 children)
              12: superseding promote_rule(::Type, ::Type) @ Base promotion.jl:319 with MethodInstance for promote_rule(::Type{Int64}, ::Type) (6 children)

defined there:

promote_rule(::Type{S}, ::Type{T}) where {S<:NumberNotSeries,T<:AbstractSeries} =
promote_rule(T,S)

Do you think we could do somehting about it ? I admit that I dont know what to do with this :)

Code I ran
using SnoopCompileCore
invalidations = @snoopr using Distributions, Copulas
tinf = @snoopi_deep begin
    biv_cops = [
        GaussianCopula([1 0.7; 0.7 1]),
        TCopula(2,[1 0.7; 0.7 1]),
        ClaytonCopula(2,7),
        JoeCopula(2,3),
        GumbelCopula(2,8),
        FrankCopula(2,0.5),
        AMHCopula(2,0.7)]
    for C in biv_cops
        u = rand(C,10)
        pdf(C,[0.5,0.5])
        cdf(C,[0.5,0.5])
        D = SklarDist(C,[Gamma(1,1),Normal(1,1)])
        u = rand(D,10)
        pdf(D,[0.5,0.5])
        cdf(D,[0.5,0.5])
    end

    X₁ = Gamma(2,3)
    X₂ = Pareto()
    X₃ = LogNormal(0,1)
    C = ClaytonCopula(3,0.7) # A 3-variate Clayton Copula with θ = 0.7
    D = SklarDist(C,(X₁,X₂,X₃)) # The final distribution
    simu = rand(D,1000)
    D̂ = fit(SklarDist{FrankCopula,Tuple{Gamma,Normal,LogNormal}}, simu)
end
using SnoopCompile
trees = invalidation_trees(invalidations)
staletrees = precompile_blockers(trees, tinf)

@show length(uinvalidated(invalidations))  # show total invalidations

show(trees[end])  # show the most invalidating method
@lbenet
Copy link
Member

lbenet commented Nov 3, 2023

Thanks @lrnv for opening this issue and filing such a detailed report. I will have a look on it, but I will also have to understand the problem.

As a simple solution, have you tested your code with the problematic lines commented?

@lrnv
Copy link
Author

lrnv commented Nov 5, 2023

Well, there is no particularly "problematic line". What I do is simply using TaylorSeries.jl to get a derivative, particularly there:

https://github.com/lrnv/Copulas.jl/blob/95fd079798d0db97ce5848dbb6fcf3b83b71af15/src/ArchimedeanCopula.jl#L20-L26

But on the other hand I am not sure if that is a very interesting bug or if we should just dont care

@lbenet
Copy link
Member

lbenet commented Dec 2, 2023

Thanks again for opening this issue, and sorry for addressing it with such a delay.

Recently, #341 was merged and includes additions to extend Aqua-related checks. In the branch lb/iss339 I commented the lines related to the promote_rule pointed out above, which seems unnecesary at least within the tests we have here (maybe they are needed in other packages that use TaylorSeries). The convert method is needed, so I changed a bit its signature; all tests pass.

Can you test if that is enough to solve (or advance) with this issue?

In working on this, I noticed that, once IntervalArithmetic is loaded (so the corresponding pkg-extension is included), Aqua detects the occurrence of some ambiguities:

julia> ambs = Aqua.detect_ambiguities(TaylorSeries; recursive = true)
2-element Vector{Tuple{Method, Method}}:
 (^(a::TaylorN{Interval{T}}, r::S) where {T<:Real, S<:Real} @ TaylorSeriesIAExt ~/.julia/packages/TaylorSeries/l6u6g/ext/TaylorSeriesIAExt.jl:46, ^(a::TaylorN, x::Rational) @ TaylorSeries ~/.julia/packages/TaylorSeries/l6u6g/src/power.jl:50)
 (^(a::Taylor1{Interval{T}}, r::S) where {T<:Real, S<:Real} @ TaylorSeriesIAExt ~/.julia/packages/TaylorSeries/l6u6g/ext/TaylorSeriesIAExt.jl:22, ^(a::Taylor1, x::Rational) @ TaylorSeries ~/.julia/packages/TaylorSeries/l6u6g/src/power.jl:50)

I will try to solve them now; they may be the source of the problems...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants