-
-
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
Refactor "srcd/sl.jl" file #985
base: master
Are you sure you want to change the base?
Changes from all commits
efd44be
d2985c0
b06068f
5cc8f41
15273f1
b105b3e
1324428
71c9b5c
9ab9290
d45f0a7
b6ba044
cdac225
ea9aae3
7084133
a6b4c3a
160668f
9c265ee
34fc2c0
e9fa7ba
37ea1d9
87464f7
8156eca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,7 +86,7 @@ function make_compound(expr) | |
# Cannot extract directly using e.g. "getfield.(composition, :reactant)" because then | ||
# we get something like :([:C, :O]), rather than :([C, O]). | ||
composition = Catalyst.recursive_find_reactants!(expr.args[3], 1, | ||
Vector{ReactantStruct}(undef, 0)) | ||
Vector{ReactantInternal}(undef, 0)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about DSLReactant and DSLReaction? That makes clear their intended use case. |
||
components = :([]) # Becomes something like :([C, O]). | ||
coefficients = :([]) # Becomes something like :([1, 2]). | ||
for comp in composition | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,15 +2,18 @@ | |||||||||
|
||||||||||
# Function that handles variable interpolation. | ||||||||||
function esc_dollars!(ex) | ||||||||||
if ex isa Expr | ||||||||||
if ex.head == :$ | ||||||||||
return esc(:($(ex.args[1]))) | ||||||||||
else | ||||||||||
for i in 1:length(ex.args) | ||||||||||
ex.args[i] = esc_dollars!(ex.args[i]) | ||||||||||
end | ||||||||||
# If we do not have an expression: recursion has finished and we return the input. | ||||||||||
isaacsas marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
(ex isa Expr) || (return ex) | ||||||||||
|
||||||||||
# If we have encountered an interpolation, perform the appropriate modification, else recur. | ||||||||||
if ex.head == :$ | ||||||||||
return esc(:($(ex.args[1]))) | ||||||||||
else | ||||||||||
for i in eachindex(ex.args) | ||||||||||
ex.args[i] = esc_dollars!(ex.args[i]) | ||||||||||
end | ||||||||||
end | ||||||||||
|
||||||||||
ex | ||||||||||
end | ||||||||||
|
||||||||||
|
@@ -22,28 +25,23 @@ end | |||||||||
### Parameters/Species/Variables Symbols Correctness Checking ### | ||||||||||
|
||||||||||
# Throws an error when a forbidden symbol is used. | ||||||||||
function forbidden_symbol_check(v) | ||||||||||
!isempty(intersect(forbidden_symbols_error, v)) && | ||||||||||
error("The following symbol(s) are used as species or parameters: " * | ||||||||||
((map(s -> "'" * string(s) * "', ", | ||||||||||
intersect(forbidden_symbols_error, v))...)) * | ||||||||||
"this is not permitted.") | ||||||||||
nothing | ||||||||||
function forbidden_symbol_check(sym) | ||||||||||
isempty(intersect(forbidden_symbols_error, sym)) && return | ||||||||||
used_forbidden_syms = intersect(forbidden_symbols_error, sym) | ||||||||||
Comment on lines
+29
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
error("The following symbol(s) are used as species or parameters: $used_forbidden_syms, this is not permitted.") | ||||||||||
end | ||||||||||
|
||||||||||
# Throws an error when a forbidden variable is used (a forbidden symbol that is not `:t`). | ||||||||||
function forbidden_variable_check(v) | ||||||||||
!isempty(intersect(forbidden_variables_error, v)) && | ||||||||||
error("The following symbol(s) are used as variables: " * | ||||||||||
((map(s -> "'" * string(s) * "', ", | ||||||||||
intersect(forbidden_variables_error, v))...)) * | ||||||||||
"this is not permitted.") | ||||||||||
function forbidden_variable_check(sym) | ||||||||||
isempty(intersect(forbidden_variables_error, sym)) && return | ||||||||||
used_forbidden_syms = intersect(forbidden_variables_error, sym) | ||||||||||
Comment on lines
+36
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
error("The following symbol(s) are used as variables: $used_forbidden_syms, this is not permitted.") | ||||||||||
end | ||||||||||
|
||||||||||
# Checks that no symbol was sued for multiple purposes. | ||||||||||
function unique_symbol_check(syms) | ||||||||||
allunique(syms) || | ||||||||||
error("Reaction network independent variables, parameters, species, and variables must all have distinct names, but a duplicate has been detected. ") | ||||||||||
nothing | ||||||||||
allunique(syms) && return | ||||||||||
error("Reaction network independent variables, parameters, species, and variables must all have distinct names, but a duplicate has been detected. ") | ||||||||||
Comment on lines
+43
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want to do this here that is fine, but generally let's try to avoid rewriting code that uses |
||||||||||
end | ||||||||||
|
||||||||||
### Catalyst-specific Expressions Manipulation ### | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,7 +131,7 @@ let | |
end | ||
# Line number nodes aren't ignored so have to be manually removed | ||
Base.remove_linenums!(ex) | ||
@test eval(Catalyst.make_reaction_system(ex)) isa ReactionSystem | ||
@test eval(Catalyst.make_reaction_system(ex, QuoteNode(:name))) isa ReactionSystem | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. adapted to the new |
||
end | ||
|
||
# Miscellaneous interpolation tests. Unsure what they do here (not related to DSL). | ||
|
@@ -211,11 +211,11 @@ let | |
end | ||
|
||
# Checks that repeated metadata throws errors. | ||
let | ||
@test_throws LoadError @eval @reaction k, 0 --> X, [md1=1.0, md1=2.0] | ||
@test_throws LoadError @eval @reaction_network begin | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was never supposed to be a |
||
k, 0 --> X, [md1=1.0, md1=1.0] | ||
end | ||
let | ||
@test_throws Exception @eval @reaction k, 0 --> X, [md1=1.0, md1=2.0] | ||
@test_throws Exception @eval @reaction_network begin | ||
k, 0 --> X, [md1=1.0, md1=1.0] | ||
end | ||
end | ||
|
||
# Tests for nested metadata. | ||
|
@@ -267,6 +267,24 @@ let | |
@test isequal(rn1,rn2) | ||
end | ||
|
||
# Tests that erroneous metadata declarations yields errors. | ||
let | ||
# Malformed metadata/value separator. | ||
@test_throws Exception @eval @reaction_network begin | ||
d, X --> 0, [misc=>"Metadata should use `=`, not `=>`."] | ||
end | ||
|
||
# Malformed lhs | ||
@test_throws Exception @eval @reaction_network begin | ||
d, X --> 0, [misc,description=>"Metadata lhs should be a single symbol."] | ||
end | ||
|
||
# Malformed metadata separator. | ||
@test_throws Exception @eval @reaction_network begin | ||
d, X --> 0, [misc=>:misc; description="description"] | ||
end | ||
end | ||
|
||
### Other Tests ### | ||
|
||
# Test floating point stoichiometry work. | ||
|
@@ -344,4 +362,4 @@ let | |
rx = Reaction(k[1]*a+k[2], [X[1], X[2]], [Y, C], [1, V[1]], [V[2] * W, B]) | ||
@named arrtest = ReactionSystem([rx], t) | ||
arrtest == rn | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,65 @@ end | |
|
||
## Run Tests ### | ||
|
||
# Tests the various network constructors. Test for `@network_component` and `@network_component`. | ||
# Tests for combinations of reactions/no reactions, no name/name/interpolated name. | ||
let | ||
# Declare comparison networks programmatically. | ||
@parameters d | ||
@species X(t) | ||
rx = Reaction(d, [X], []) | ||
|
||
rs_empty = ReactionSystem([], t; name = :name) | ||
rs = ReactionSystem([rx], t; name = :name) | ||
rs_empty_comp = complete(rs_empty) | ||
rs_comp = complete(rs) | ||
|
||
# Declare empty networks. | ||
name_sym = :name | ||
rs_empty_1 = @network_component | ||
rs_empty_2 = @network_component name | ||
rs_empty_3 = @network_component $name_sym | ||
rs_empty_comp_1 = @reaction_network | ||
rs_empty_comp_2 = @reaction_network name | ||
rs_empty_comp_3 = @reaction_network $name_sym | ||
|
||
# Check that empty networks are correct. | ||
isequivalent(rs_empty_1, rs_empty) | ||
rs_empty_2 == rs_empty | ||
rs_empty_3 == rs_empty | ||
isequivalent(rs_empty_comp_1, rs_empty_comp) | ||
rs_empty_comp_2 == rs_empty_comp | ||
rs_empty_comp_3 == rs_empty_comp | ||
|
||
# Declare non-empty networks. | ||
rs_1 = @network_component begin | ||
d, X --> 0 | ||
end | ||
rs_2 = @network_component name begin | ||
d, X --> 0 | ||
end | ||
rs_3 = @network_component $name_sym begin | ||
d, X --> 0 | ||
end | ||
rs_comp_1 = @reaction_network begin | ||
d, X --> 0 | ||
end | ||
rs_comp_2 = @reaction_network name begin | ||
d, X --> 0 | ||
end | ||
rs_comp_3 = @reaction_network $name_sym begin | ||
d, X --> 0 | ||
end | ||
|
||
# Check that non-empty networks are correct. | ||
isequivalent(rs_1, rs) | ||
rs_2 == rs | ||
rs_3 == rs | ||
isequivalent(rs_empty_1, rs_empty) | ||
rs_empty_2 == rs_empty | ||
rs_empty_3 == rs_empty | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WHen I remade how the macor inputs are handled at the top level (nothing, name, reactions, name + reactions) I wanted to add some tests that all of this works for all cases and both macros. |
||
|
||
# Test basic properties of networks. | ||
let | ||
basic_test(reaction_networks_standard[1], 10, [:X1, :X2, :X3], | ||
|
@@ -404,7 +463,7 @@ let | |
@test rn1 == rn2 | ||
end | ||
|
||
# Tests arrow variants in `@reaction`` macro . | ||
# Tests arrow variants in `@reaction`` macro. | ||
let | ||
@test isequal((@reaction k, 0 --> X), (@reaction k, X <-- 0)) | ||
@test isequal((@reaction k, 0 --> X), (@reaction k, X ⟻ 0)) | ||
|
@@ -437,6 +496,41 @@ let | |
@test_throws LoadError @eval @reaction nothing, 0 --> B | ||
@test_throws LoadError @eval @reaction k, 0 --> im | ||
@test_throws LoadError @eval @reaction k, 0 --> nothing | ||
|
||
# Checks that non-supported arrow type usage yields error. | ||
@test_throws Exception @eval @reaction_network begin | ||
d, X ⇻ 0 | ||
end | ||
end | ||
|
||
### Error Test ### | ||
|
||
# Erroneous `@reaction` usage. | ||
let | ||
# Bi-directional reaction using the `@reaction` macro. | ||
@test_throws Exception @eval @reaction (k1,k2), X1 <--> X2 | ||
|
||
# Bundles reactions. | ||
@test_throws Exception @eval @reaction k, (X1,X2) --> 0 | ||
end | ||
|
||
# Tests that malformed reactions yields errors. | ||
let | ||
# Checks that malformed combinations of entries yields errors. | ||
@test_throws Exception @eval @reaction_network begin | ||
d, X --> 0, Y --> 0 | ||
end | ||
@test_throws Exception @eval @reaction_network begin | ||
d, X --> 0, [misc="Ok metadata"], [description="Metadata in (erroneously) extra []."] | ||
end | ||
|
||
# Checks that incorrect bundling yields error. | ||
@test_throws Exception @eval @reaction_network begin | ||
(k1,k2,k3), (X1,X2) --> 0 | ||
end | ||
|
||
# Checks that incorrect stoichiometric expression yields an error. | ||
@test_throws Exception @eval @reaction_network begin | ||
k, X^Y --> XY | ||
end | ||
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.
this function is used frequenty, and even more frequently when debugging code. Figured it made sense to import directly.