-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Update inference of variables and default differential from @equations
macro
#1175
Conversation
@equations
macro@equations
macro
src/dsl.jl
Outdated
species_extracted, parameters_extracted = extract_species_and_parameters!( | ||
reactions, declared_syms; requiredec) | ||
|
||
# Reads equations (and infers potential variables). | ||
# Exlucdes any parameters already extracted (if they also was a variable). | ||
designated_syms = [declared_syms; species_extracted] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update designated_syms
with species_extracted
so these are excluded from the next step.
@@ -97,9 +97,9 @@ Base.@kwdef mutable struct NetworkProperties{I <: Integer, V <: BasicSymbolic{Re | |||
stronglinkageclasses::Vector{Vector{Int}} = Vector{Vector{Int}}(undef, 0) | |||
terminallinkageclasses::Vector{Vector{Int}} = Vector{Vector{Int}}(undef, 0) | |||
|
|||
checkedrobust::Bool = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to fix this thing. I have checked an re-checked and there is no actual difference (more than space removal). I think I put in, and then removed, a debug statement in this file (and the space removal happened). Hence changes appeared here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you must have some VSCode setting enabled.
@equations
macro@equations
macro
- New formula for determining whether the default differentials have been used within an `@equations` option. Now, if any expression `D(...)` is encountered (where `...` can be anything), this is inferred as usage of the default differential D. E.g. in the following equations `D` is inferred as a differential with respect to the default independent variable: | ||
```julia | ||
@reaction_network begin | ||
@equations D(V) + V ~ 1 | ||
end | ||
@reaction_network begin | ||
@equations D(D(V)) ~ 1 | ||
end | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if someone uses D
in a reaction as an implicit species?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from that question this is fine to merge I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using D in any other context (e.g. as a species, variable or parameter) should disable inference of D as a differential. Will clarify this
@@ -97,9 +97,9 @@ Base.@kwdef mutable struct NetworkProperties{I <: Integer, V <: BasicSymbolic{Re | |||
stronglinkageclasses::Vector{Vector{Int}} = Vector{Vector{Int}}(undef, 0) | |||
terminallinkageclasses::Vector{Vector{Int}} = Vector{Vector{Int}}(undef, 0) | |||
|
|||
checkedrobust::Bool = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you must have some VSCode setting enabled.
Turns out the docs have started failing, something in the oscillation parameter fitting example. |
Updates what we infer as variables from equations, and also when we detect that the default differential
D
is used. The new formula is a bit more liberal, which will make describing it a bit easier.Inferring variables
The order of inference of species/variables/parameters is now:
(1) Every symbol explicitly declared using
@species
,@variables
, and@parameters
are assigned to the correct category.(2) Every symbol used as a reaction reactant is inferred as a species.
(3) Every symbol not declared in (1) or (2) that occurs in an expression provided after
@equations
is inferred as a variable.(4) Every symbol not declared in (1), (2), or (3) that occurs either as a reaction rate or stoichiometric coefficient is inferred to be a parameter.
E.g. in
S
is inferred as a species,V1
andV2
as variables, andp
as a parameter.The previous special cases for the
@observables
(if not declared as symbol/variable, becomes a variable without additional info),@compounds
(counts as@species
declarations), and@differentials
(used to declare non-D differentials) still hold.Finally, we also now have the
@require_declaration
options which can be used to remove any uncertainty in messy cases.Inferring use of differential
D
When determining whether the default differentials have been used within an
@equations
option. Now, if any expressionD(...)
is encountered (where...
can be anything), this is inferred as usage of the default differential D. E.g. in the following equationsD
is inferred as a differential with respect to the default independent variable:The code handling is getting a bit messier than it has to be. This will be better in the PR which finally fixes the mess that is the DSL file. Primarily we will probably just create lists for stuff declared using
@species
,@variables
, and@parameters
and then which is inferred. And then this can just be processed in an orderly manner. But for now, this does all the breaking changes, which we want with v15.I have also added a couple of tests for good measure.