Skip to content
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

aqua CI and related fixes #389

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

aqua CI and related fixes #389

wants to merge 6 commits into from

Conversation

isaacsas
Copy link
Member

@isaacsas isaacsas commented Jan 3, 2024

Adds Aqua test, seems to indicate that #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.

@isaacsas isaacsas closed this Jan 3, 2024
@isaacsas isaacsas reopened this Jan 3, 2024
@isaacsas
Copy link
Member Author

isaacsas commented Jan 3, 2024

@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?

@isaacsas
Copy link
Member Author

isaacsas commented Jan 3, 2024

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.

@Vilin97
Copy link
Contributor

Vilin97 commented Jan 7, 2024

I fixed the ambiguities coming from spatial stuff in #392. Once this PR is merged, #392 can be reviewed and merged as well.

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 this pull request may close these issues.

None yet

3 participants