Invalidation of nameof

[oxinabox]

So I was going through with SnoopCompile trying to workout why even with a precompilation script i was still seeing 90+% of my time spent on recompilation.

Seems like Catalyst does a lot of invalidations.
the following seems reasonable to fix:

julia> trees[end-1]
inserting nameof(lrs::Catalyst.LatticeReactionSystem) @ Catalyst ~/.julia/packages/Catalyst/DMVHG/src/spatial_reaction_systems/lattice_reaction_systems.jl:484 invalidated:
   backedges: 1: superseding nameof(sys::ModelingToolkit.AbstractSystem) @ ModelingToolkit ~/.julia/packages/ModelingToolkit/aiAqm/src/systems/abstractsystem.jl:218 with MethodInstance for nameof(::ModelingToolkit.AbstractSystem) (2093 children)

This is the single biggest invalidation,
and it could be fixed by not overloading this,
and instead just setting the name field during construction.
https://github.com/SciML/ModelingToolkit.jl/blob/279ec69a3dc5ffaaa3660448361c07dd9783b3c7/lib/ModelingToolkitBase/src/systems/abstractsystem.jl#L83

Most of the others i suspect will be fixed by getting on to the lastest verson of MTK and SymbolicUtils.jl

[isaacsas]

@TorkelE can you look into this?

More generally, this would be a reason for my (previously made) suggestion that we consider moving some non-core functionality to separate libraries (particularly network analysis components and spatial components).

Perhaps we should have a CatalystBase, and that way Catalyst can stay the meta package with everything, but CatalystBase can be the slim component with just the basics for system construction and conversion?

[TorkelE]

Can have al ook, think this is just me not understanding Julai well enough though. When I created the LatticeReactionSystem I though the idea was to have all functions applicable to normal systems be applicable to that as well, so that I had to define nameof for it. But somehow the way I did so was wrong?

[ChrisRackauckas]

Just delete the function and make a .name field that's a symbol.

[oxinabox]

But somehow the way I did so was wrong?

No. The invalidation rules in julia are an unfortunately reality of the language implementation.
And likely not one that is possibly to avoid, at least not without cutting out a useful class of optimizations, or getting rid of all precompilation (in which case it would only crop-up during interactive sessions, or other things that combine eval and invoke-latest.).
But by a normal users understanding of the language (rather than the implementation), you have done nothing wrong.
But in large dependancy projects like Catalyst, it is something that needs to be considered.

Basically, if there is only one overload for any foo(::AbstractT) then julia will staticly dispatch (or outright inline) to that definition into places that use it. (Also applies if there are 4 or less due to union splitting).
Even if inference can't workout what particular subtype of AbstractT it is working with -- since it doesn't need to, there is only one method to hit regardless.
By introducing another one, the code that has those static dispatches has to change. (either to insert a union-splitting style branch between the general case and the 1 specific case. Or a dynamic dispatch).
So it gets "invalidated".
And needs to be recompiled next time it is called.

Anyway, ideally this would be handled by the callling code. But this particular case is so easy to fix in the defining code.

[TorkelE]

agree, this is such a tangential function that I wouldn't be suprised if no one has actually ever called it, will sort this one out. Also, thanks for running checks like this one the package, a great help!

[TorkelE]

I have tried jsut adding the name field,

    # Other values.
    "The name of the discrete space reaction system. Typically taken directly from the base `ReactionSystem`."
    name::Symbol

but there seems to be issues with this for this type?

        
julia> typeof(dsrs)
DiscreteSpaceReactionSystem{Catalyst.NetworkProperties{Int64, SymbolicUtils.BasicSymbolicImpl.var"typeof(BasicSymbolicImpl)"{SymReal}}, TransportReaction, Matrix{Bool}, Vector{Pair{Int64, Int64}}}

        
julia> fieldnames(typeof(dsrs))
(:reactionsystem, :spatial_reactions, :dspace, :num_verts, :num_edges, :num_species, :spatial_species, :parameters, :vertex_parameters, :edge_parameters, :edge_iterator, :name)

julia> getfield(dsrs, :name)
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1

julia> getproperty(dsrs, :name)
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] nameof
   @ C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:83 [inlined]
 [2] getvar(sys::DiscreteSpaceReactionSystem{…}, name::Symbol; namespace::Bool)
   @ ModelingToolkitBase C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1133
 [3] getvar
   @ C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1076 [inlined]
 [4] getproperty(sys::DiscreteSpaceReactionSystem{…}, name::Symbol; namespace::Bool)
   @ ModelingToolkitBase C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1074
 [5] getproperty(sys::DiscreteSpaceReactionSystem{…}, name::Symbol)
   @ ModelingToolkitBase C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1068
 [6] top-level scope
   @ REPL[4]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> getfield(dsrs, :reactionsystem)
Model customname:
Unknowns (1): see unknowns(customname)
  X(t)
Parameters (1): see parameters(customname)
  p

julia> getproperty(dsrs, :reactionsystem)
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] nameof
   @ C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:83 [inlined]
 [2] getvar(sys::DiscreteSpaceReactionSystem{…}, name::Symbol; namespace::Bool)
   @ ModelingToolkitBase C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1133
 [3] getvar
   @ C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1076 [inlined]
 [4] getproperty(sys::DiscreteSpaceReactionSystem{…}, name::Symbol; namespace::Bool)
   @ ModelingToolkitBase C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1074
 [5] getproperty(sys::DiscreteSpaceReactionSystem{…}, name::Symbol)
   @ ModelingToolkitBase C:\Users\Torkel\.julia\packages\ModelingToolkitBase\JOBPV\src\systems\abstractsystem.jl:1068
 [6] top-level scope
   @ REPL[6]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> supertype(typeof(dsrs))
ModelingToolkitBase.AbstractSystem