Catalyst.jl
Catalyst.jl copied to clipboard
[v14 ready] Implement metadata for `Reactions`s
The Reaction
type now have a .metadata
field:
"""
Contain additional data, such whenever the reaction have a specific noise-scaling expression for
the chemical Langevin equation.
"""
metadata::Dict{Symbol, R}
I would have used Base.ImmutableDict
s, but there are problems with regards to these: https://discourse.julialang.org/t/create-immutabledict-with-values-of-differenrt-types/107831
The metadata can be accessed via the following functions:
get_metadata_dict
has_metadata
get_metadata
Metadata can either be added as an optional argument when creating Reactions
s programmatically:
using Catalyst
@variables t
@parameters k
@species X(t) X2(t)
metadata = Dict(:noise_scaling => 0.0)
r = Reaction(k, [X], [X2], [2], [1]; metadata=metadata)
Here, the accessors works like this:
get_metadata_dict(r)
has_metadata(r, :noise_scaling)
get_metadata(r, :noise_scaling)
Metadata can also be provided via the DSLs:
rs = @reaction_network begin
@parameters η
k, 2X --> X2, [noise_scaling=η]
k, 2X --> X2, [some_metadata=1.0, more_metadata="Hello world"]
end
@parameters η
@reaction k, 2X --> X2, [noise_scaling=$η]
@reaction k, 2X --> X2, [some_metadata=1.0, more_metadata="Hello world"]
Finally, instead of using arrows like =>
,
rs = @reaction_network begin
k, 2X --> X2, [only_use_rate=true]
end
can be used to remove the substrate expression from the rate. Internally, only_use_rate=true
is carried through as a metadata to record this. However, it is removed at the end.
Currently, no additional metadata is actually used. However, noise_scaling
will be added after this is merged.
Also:
- I have updated this, adding
Bool
:const ExprValues = Union{Expr, Symbol, Float64, Int, Bool}
. Here,Bool
s can potentially be inside expressions (likeFloat64
). We missed this before and I realised when working with this. Added it now. - Since some tests here are specific to the
Reaction
struct, I added another test file for tests related to this struct. - I made a minor internal remake so that reactions created in
@reaction
and@reaction_network
are created using the same internal function.
Considerations:
- Should
only_use_rate
remain in the metadata post-creation? This could also be entirely turned into a metadata thing. Alternatively, we keep it as currently implemented, where this can be used as an option, but never actually appear in the metadata. - Should we maintain a list of permitted metadata? This would prevent the user to add their own, but would give us more control. Alternatively, we can add a warning if another metadata was used. This would be useful if someone e.g. writers
noise_scalling =
, since we could then catch this error.
It probably doesn't make too much of a difference at this level, but I think the continuity of having it match Symbolics and ModelingToolkit's metadata system would make things cleaner.
At some point that might make sense. Currently, we do not have any metadata to manage though. I'd suggest merging this and https://github.com/SciML/Catalyst.jl/pull/678. Then at a later stage when we know what we actually want from reaction metadata we can consider implementing a similar system.
I don’t think we should use Dicts though. That seems pretty heavy for when we have systems with thousands of reactions. Looking at that thread I’d say we are in the use case for ImmutableDicts (small numbers of objects being stored). And we should settle the interface now, otherwise it will end up a breaking change in the future.
Alternatively we could just use a flat vector and linear search given we don’t expect lots of metadata.
If we go down the MTK route, we are still happy with the other parts, especially how only_use_rate
is handled?
Is there a good resource on the MTK thing? I don't dislike the idea of using that interface, but I don't want to get bogged down and spend the next couple of days looking at a very niche implementation that we still rate not sure will actually matter within the next couple of years (especially since both I and Catalyst have better stuff to concentrate on).
Basically, if we want to go down the MTK route I would want to know what we want and how now, rather than learning details over 3 different revisions of a PR.
Another alternative should be to simply make it a named Tuple? That would allow indexing of the fields, make it immutable, and be good for storage? That should work equivalently and also be relatively light weigth.
The last line here is what causes the error:
@variables t
@parameters k
@species X(t) X2(t)
metadata = [:md_1 => 1.0, :md_2 => false, :md_3 => "Hello world", :md_4 => :sym, :md_5 => X + X2^k -1, :md_6 => (0.1, 2.0)]
No idea why, it all works fine locally.
Ok, this one is ready now.
I had a discussion with Shashi, who suggested this. There are some special reasons why Symbolics have to do it the way they do it. However, they do not apply to Catalyst, hence this interface can be used (which is way simpler).
Can write more extensively, or discuss in person, later.
The reason for Symbolic's interface doesn't really matter here though -- what matters is that now we are using a completely different interface. (I'm not talking about the internal implementation details but the user facing way to add new metadata, query its existence as an option, and get its value when present in a given Reaction
.) However we internally implement the details why not make the external interface for declaring a new metadata type, querying existence of a given type within a reaction, and getting values of a specific type of metadata the same?
On the airport right now, but will think more on it when I get back.
While I see the advantage of having the same interface, this one is just insanely much easier to use. The Symbolics one feels really internal, and is nothing I see anyone using outside of package development.
If you want we can simply remove the doc part of this one, and let it be internal for now though.
Letäs discuss this one in person, I am still a bit uncertain exactly what you envision would be removed/added/external/internal.
This is now ready @isaacsas
@TorkelE once these are addressed we can merge.