Handle units #798
Handle units #798
Conversation
|
This PR is based on #784, hence the number if displayed commits. |
|
Look at how ModelingToolkit imports them both. It seems to only import dynamic quantities. |
|
Will have a look at MTK, that makes sense. Seems like something that should be solvable |
|
I've added some additional improvements. Have tried some more, and still have not gotten Unitful to work. Hopefully SymbolicML/DynamicQuantities.jl#128 gets merged soon, when it do we will have molar units (M) as well. |
|
Molar is fixed in DynamicQuantities now. Unfortunately, MTK still do not accept it (SciML/ModelingToolkit.jl#2590). |
TorkelE
force-pushed
the
handle_units
branch
2 times, most recently
from
f0aa3b3
to
a90eda7
Compare
| @@ -302,6 +302,7 @@ let | |||
|
|
|||
| for repeat in 1:5 | |||
| sol = solve(sprob, ImplicitEM(); saveat = 1.0, adaptive = false, dt = 0.01, seed = rand(rng, 1:100)) | |||
| SciMLBase.successful_retcode(sol) || continue | |||
There was a problem hiding this comment.
Rarely, but it does happen, StochasticDiffEq fails to initiate the problem properly, giving a NaN solution and a failed test. Added this one so we should not have to worry about it.
(I am surprised though that it caused a failure in this run, but not for the last PR, given that new change was made to this file since then)
There was a problem hiding this comment.
But shouldn't this be deterministic given we are using StableRNGs with a fixed seed? (i.e. if we pick a seed where it doesn't error then it never should.)
| @@ -302,6 +302,7 @@ let | |||
|
|
|||
| for repeat in 1:5 | |||
| sol = solve(sprob, ImplicitEM(); saveat = 1.0, adaptive = false, dt = 0.01, seed = rand(rng, 1:100)) | |||
| SciMLBase.successful_retcode(sol) || continue | |||
There was a problem hiding this comment.
But shouldn't this be deterministic given we are using StableRNGs with a fixed seed? (i.e. if we pick a seed where it doesn't error then it never should.)
| sol_base = solve(jprob_base, SSAStepper(); seed, saveat = 1.0) | ||
| sol_alt = solve(jprob_alt, SSAStepper(); seed, saveat = 1.0) | ||
| sol_base == sol_alt | ||
| sol_alt1 = solve(jprob_alt1, SSAStepper(); seed, saveat = 1.0) | ||
| sol_alt2 = solve(jprob_alt2, SSAStepper(); seed, saveat = 1.0) | ||
| @test_broken sol_base == sol_alt1 == sol_alt2 |
Co-authored-by: Sam Isaacson <isaacsas@users.noreply.github.com>
This updates Catalyst to properly handle units in v14 (when depending on MTK v9).
Currently, Molar units are not supported by dynamic quantities (SymbolicML/DynamicQuantities.jl#127), so I just used
min the test (which is weird, but should work equivalently well until we can replace it with something sensibly-looking).When I tried
using DynamicQuantities, Unitfulin the main Catalyst.jl source file, things stopped working, so I have currently commented out Unitful. It is possible that we might be able to support it, but I will have to look further at that.