Add additional Jacobian tests (including sparsity) #1211
Conversation
|
This is the likely culprit SciML/OrdinaryDiffEq.jl#2567 |
| end | ||
|
|
||
| # Checks that the Jacobians (dense and sparse) are identical for different system types. |
There was a problem hiding this comment.
Do you ever test against a custom hand-coded Jacobian we know is right? That would be good to have in at least one case.
There was a problem hiding this comment.
Yes we do (right now that's basically the only Jacobian tests we do actually): https://github.com/SciML/Catalyst.jl/blob/master/test/simulation_and_solving/jacobian_construction.jl
However, we do not do this for sparse though.
There was a problem hiding this comment.
We should check sparse against the exact Jacobian, but I don't think we need an "exact" Jacobian that returns a sparse matrix (i.e. checking the Catalyst-generated sparse vs. a hand dense is fine).
|
Feel free to merge if tests pass and you are happy. |
|
I wouldn't worry too much about pre since it's now actually on the pre-release, though that fail is odd . Let blocks are stronger scoped in the next release because of the globals changes, and so we've seen that causes changes in eval / RGF handling. I don't see why that would be related here because I don't see eval at all though... |
After the latest test failures in the spatial tests I checked and realised that we don't actually properly test whether the generated sparse Jacobians are correct. I guess, ironically, a lot of the core feature tests were written way back in time before when we made more assumptions of upstream correctness (and our own, I guess, still think the early tests were quite good though). Hence a lot of stuff is actually caught in extensions/niche feature tests that are actually testing other stuff.
I haven't actually isolated the latest problems, so not sure if they are from Catalyst or somewhere else (or even if they are a problem). However, we get failures here as well, so not related to the spatial stuff that is actually failing. I also only get failures of these tests on the latest version, so it is something recent that has happened.