Expand conservation law tests #907
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TorkelE
commented
Jun 4, 2024
| @@ -653,7 +653,7 @@ function Base.convert(::Type{<:JumpSystem}, rs::ReactionSystem; name = nameof(rs | |||
| spatial_convert_err(rs::ReactionSystem, JumpSystem) | |||
|
|
|||
| (remove_conserved !== nothing) && | |||
| error("Catalyst does not support removing conserved species when converting to JumpSystems.") | |||
| throw(ArgumentError("Catalyst does not support removing conserved species when converting to JumpSystems.")) | |||
There was a problem hiding this comment.
Ensuring that this is a ArgumentError allow us to write a better test later on (since we can test for that as opposed to Exception, which could be generated by other stuff).
TorkelE
commented
Jun 4, 2024
| @@ -71,7 +71,7 @@ let | |||
| Set([:p, :k1, :k2, :k3, :k4, :k5, :k6, :d]) | |||
| basic_test(reaction_networks_hill[1], 4, [:X1, :X2], | |||
| [:v1, :v2, :K1, :K2, :n1, :n2, :d1, :d2]) | |||
| basic_test(reaction_networks_constraint[1], 6, [:X1, :X2, :X3], | |||
| basic_test(reaction_networks_conserved[1], 6, [:X1, :X2, :X3], | |||
There was a problem hiding this comment.
I named the reaction_networks_constraint networks that when I didn't really know about the terminology that well. Changde the names to something better.
TorkelE
commented
Jun 4, 2024
| (k1, k2), X1 ↔ X2 | ||
| (k3, k4), X2 ↔ X3 | ||
| (k5, k6), X3 ↔ X1 | ||
| end | ||
| reaction_network_constraints[1] = [1 1 1] | ||
| reaction_network_conslaws[1] = [1 1 1] |
TorkelE
commented
Jun 4, 2024
| sol2 = solve(oprob2; saveat = 1.0) | ||
| @test all(sol1[osys.X1 + osys.X2] .== 2.0) | ||
| @test all(sol2[osys.X1 + osys.X2] .== 4.0) | ||
| end |
There was a problem hiding this comment.
I also wanted to have a test checking
rn = @reaction_network begin
@parameters k[1:2]
@species X(t)[1:2]
(k[1],k[2]), X[1] <--> X[2]
endwhen conservation laws are eliminated (would be cool if it worked, but think there's an error). However, testing this took really long time. Might be related to the problem we saw earlier, but not sure.
isaacsas
approved these changes
Jun 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds additional tests of conservation laws, especially relating to when attempting to simulate them.
Only non-test change is that
now throws an
ArgumentError(as opposed to a generic error). The reason was that this was I could write a test to check that it does throw a specific error (and not just one from wrong input).