Catalyst.jl icon indicating copy to clipboard operation
Catalyst.jl copied to clipboard

Needed test updates

Open isaacsas opened this issue 10 months ago • 2 comments

  1. We need to stop assuming ordering of unknowns/ps in tests. I tried to eliminate this in the main library long ago, but there are apparently still some tests that have issues in this regard.
  2. We should test that ps and us are subsets of system ps and us in general, and not test set equality (since MTK may now add extra stuff). When using structural_simplify we need to test vs. us and observed since us can be moved to observed.
  3. Some tests around conservation laws may be incorrect, see Aayush's Slack comment that:
https://github.com/SciML/Catalyst.jl/blob/master/test/upstream/mtk_structure_indexing.jl#L405
This test is incorrect. prob9 has the initial conditions Gamma[1] => 10.0, X1 => 10.0, X2 => 2.0 which are inconsistent since X1 + X2 ~ Gamma[1] (edited) 
[9:29](https://julialang.slack.com/archives/C0894545874/p1745501399505049)
The same holds for Gamma[2] => 20.0, Y1 => 3.0, Y2 => 4.0

isaacsas avatar Apr 24 '25 13:04 isaacsas

  1. I think I did a long pass for this regarding inputs, but this seems to now become a thing regarding outputs as well. I will have a look at it.
  2. I'm trying to clarify this with Aayush. The tests are good I think, but we should do stuff to remove the warning as previously doiscussed.

TorkelE avatar Apr 24 '25 14:04 TorkelE

The output thing is something we need to deal with more generally whenever structural_simplify has been involved. However, there are some implications for plotting I'd like to have ironed out first ideally, and I'd like to see what actually is going to happen in MTKv10 as well, as it feels like that is changing things up drastically.

TorkelE avatar Apr 24 '25 14:04 TorkelE