JumpProcesses.jl
JumpProcesses.jl copied to clipboard
aqua CI and related fixes
Adds Aqua test, seems to indicate that https://github.com/SciML/JumpProcesses.jl/issues/384 is fixed.
@Vilin97 the Aqua test is giving a bunch of method ambiguities coming from the spatial mass action jump constructors. I think the issue is that you have overlapping definitions because of the union type you are using (i.e. multiple constructors cover the case that an input is typeNothing
).
method_ambiguity = (kwcall(::NamedTuple, ::Type{SpatialMassActionJump}, srates::B, rs, ns, pmapper) where B<:Union{Nothing, AbstractMatrix} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:61, kwcall(::NamedTuple, ::Type{SpatialMassActionJump}, urates::A, rs, ns, pmapper) where A<:Union{Nothing, AbstractVector} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:73)
method_ambiguity = (SpatialMassActionJump(urates::A, srates::B, rs, ns; scale_rates, useiszero, nocopy) where {A<:Union{Nothing, AbstractVector}, B<:Union{Nothing, AbstractMatrix}} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:53, SpatialMassActionJump(srates::B, rs, ns, pmapper; scale_rates, useiszero, nocopy) where B<:Union{Nothing, AbstractMatrix} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:61)
method_ambiguity = (SpatialMassActionJump(srates::B, rs, ns; scale_rates, useiszero, nocopy) where B<:Union{Nothing, AbstractMatrix} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:67, SpatialMassActionJump(urates::A, rs, ns; scale_rates, useiszero, nocopy) where A<:Union{Nothing, AbstractVector} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:79)
method_ambiguity = (SpatialMassActionJump(srates::B, rs, ns, pmapper; scale_rates, useiszero, nocopy) where B<:Union{Nothing, AbstractMatrix} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:61, SpatialMassActionJump(urates::A, rs, ns, pmapper; scale_rates, useiszero, nocopy) where A<:Union{Nothing, AbstractVector} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:73)
method_ambiguity = (kwcall(::NamedTuple, ::Type{SpatialMassActionJump}, urates::A, srates::B, rs, ns) where {A<:Union{Nothing, AbstractVector}, B<:Union{Nothing, AbstractMatrix}} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:53, kwcall(::NamedTuple, ::Type{SpatialMassActionJump}, srates::B, rs, ns, pmapper) where B<:Union{Nothing, AbstractMatrix} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:61)
method_ambiguity = (kwcall(::NamedTuple, ::Type{SpatialMassActionJump}, srates::B, rs, ns) where B<:Union{Nothing, AbstractMatrix} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:67, kwcall(::NamedTuple, ::Type{SpatialMassActionJump}, urates::A, rs, ns) where A<:Union{Nothing, AbstractVector} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:79)
method_ambiguity = (JumpSet(vj, cj, rj, maj::MassActionJump{S, T, U, V}) where {S<:Number, T, U, V} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/jumps.jl:497, JumpSet(jumps::JumpProcesses.AbstractJump...) @ JumpProcesses ~/.julia/dev/JumpProcesses/src/jumps.jl:512)
method_ambiguity = (rate_at_site(rx, site, smaj::SpatialMassActionJump{Nothing, B, S, U, V}) where {B, S, U, V} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:112, rate_at_site(rx, site, smaj::SpatialMassActionJump{A, Nothing, S, U, V}) where {A, S, U, V} @ JumpProcesses ~/.julia/dev/JumpProcesses/src/spatial/spatial_massaction_jump.jl:116)
We can fix those in a separate PR, for now I just capped the method ambiguities at 8.
@ChrisRackauckas do you have any idea why when testing locally on my Mac I only get 8 ambiguities but CI gets 35? Is this platform dependent?
I fixed the ambiguities outside of the spatial solver with the exception of the ODE_DEFAULT_NORM
one. I don't really understand why we are getting some of these ambiguities since they seem to often be arising from a version of the function defined on abstract types elsewhere, when we are specifying at least one concrete type in our method. I guess I don't quite understand dispatch rules in that case.
I fixed the ambiguities coming from spatial stuff in https://github.com/SciML/JumpProcesses.jl/pull/392. Once this PR is merged, https://github.com/SciML/JumpProcesses.jl/pull/392 can be reviewed and merged as well.
Given the wonkiness of ambiguities reported by Aqua (i.e. being platform dependent for example), and its seemingly over aggressive declaration of things as ambiguous I'm going to close this PR. Some of the fixes might make sense to put in a followup PR, but I don't think we want to add Aqua to the test suite based on what I was observing in this PR.
Should SciML decide to add Aqua to CI across the board we can resurrect this.
I'll keep the branch around so this can possibly be resurrected in the future.