Fix CI #1339
Fix CI #1339
Conversation
|
@TorkelE @devmotion let's use this as a PR to work in to get tests working and docs building. Once that is all fixed I am happy to make a release, but I don't want to make one while tests are failing and/or docs don't build. |
| sint = init(sprob, ImplicitEM(); save_everystep = false) | ||
| jint = init(jprob, SSAStepper()) | ||
| nint = init(nprob, NewtonRaphson(); save_everystep = false) | ||
| @test_broken ssint = init(ssprob, DynamicSS(Tsit5()); save_everystep = false) # https://github.com/SciML/SciMLBase.jl/issues/660 |
There was a problem hiding this comment.
@TorkelE this doesn't return a boolean or error, so the test is not broken. I assume you meant this as a flag that there is an issue, but it seems to work now. Note though that ssint is not used anymore throughout the tests, so nothing is really being tested for it now beyond init.
There was a problem hiding this comment.
This has been fine for 16 months, so I presume that something has changed underneath. I will update to ... false # ..., which should make things run for now. Then when we do the MTK fixes I will go through all of the changes and implications proiperly again.
|
AFAICT we're stuck with Arblib 1.4 which does not work on Julia >= 1.12 (fixed in Arblib >= 1.6) due to interactions with StructuralIdentifiability which is in the same environment for the Extensions tests: StructuralIdentifiability depends on FLINT_jll as well through Nemo, and Nemo pins FLINT_jll quite strictly; however, recent versions of StructuralIdentifiability which support recent versions of Nemo and hence recent versions of FLINT_jll only support MTK@10. Breaking up the Extensions environment in smaller environments with presumable less conflicts of dependencies could be a solution; alternatively, one could maybe try to backport support for recent versions of Nemo to older MTK@9 compatible releases of StructuralIdentifiability. |
|
It's surprising they could update StructuralIdentifiability to MTK10 without making a breaking release. |
|
Think SI just really uses MTK as an interface for declaring a model, so it wouldn't surprise me if no workflows have actually changed. If there has been package updates that means that things no longer work, we should introduce (as liberal as possible) compat bounds to ensure that thinsg actually works. |
|
I think the bigger issue might be that fundamentally we won't work on 1.12 with MTK9 and the versions of the extensions that work with it. It might be that we should just cap testing to 1.11 and put a note on the Catalyst homepage that we currently only support 1.11, with 1.12 support projected in the next breaking release. |
|
The hybrid test failure is a real issue that I'm trying to debug. There was a typo in the test, and once fixed the test fails as the callback is never triggered. I haven't been able to figure out why that is happening yet though. |
|
Not sure though why tests were previously passing with that typo. |
|
Maybe due to an upstream fix? |
It's really not a Catalyst issue IMO. Previous versions of Arblib had a bug. It's a problem of Arblib and a problem you'll always run into if you use Arblib < 1.6 on Julia 1.12. I don't think it should be Catalyst's responsibility to warn users about a bug caused by Arblib. To unbreak the tests, we could also try to backport this Arblib fix to a backport release of Arblib (probably 1.4.1?). |
|
I asked for the possibility of a backport release of Arblib: kalmarek/Arblib.jl#220 (comment) |
|
I marked the blocking test as broken but other hybrid tests are still failing locally and I think it is due to continuous event handling. They used to pass when we last released so something seems to have changed to cause them to be broken now in MTK or lower. |
|
Anyways I can't work more on this till next week now as I have some deadlines the next few days, but I can circle back next week hopefully to investigate the hybrid tests (or perhaps @TorkelE can look into it). It seems that continuous events no longer properly trigger for jumps over ODEProblem, which definitely worked at one point, so will require some work to investigate what is going one and where changes happened. |
|
I will try to do what I can, but the hybrid stuff looks like it can be a rather deep rabbit hole (but definitely one require solving. The issue with is simply that the broken test have now been fixed (as confirmed by the link issue). passes as any other broken test, but as There are a couple of downstream tests on |
As #1338 shows, tests currently fail on master.
The PR fixes the following test sets: