Support CairoMakie above 0.13

[pkofod]

I suppose the restriction is just there because these are breaking changes due to sub 1.0 minor changes.

Checklist

• Appropriate tests were added
• Any code changes were done in a way that does not break public API
• All documentation related to code changes were updated
• The new code follows the
  contributor guidelines, in particular the SciML Style Guide and
  COLPRAC.
• Any new documentation only uses public API

Additional context

Add any other context about the problem here.

[pkofod]

Not sure if any tests are supposed to fail here

[TorkelE]

I am not too surprised some errors have snuck in into CI over the last while. Catalyst is based on ModelingToolkit, which had a new breaking change a while ago that we have not had time to update to. In tandem with that there have been updates to MTK-related packages, it probably some change there made assumptions that means these errors happens if you are not using the latest ModelingToolkit.

Hopefully we will be able to make an update soon. A major hindrance is that the next ModelingToolkit breaking release is due very soon, so we are simply waiting for that one and skipping a step. Thanks for the PR though, we will integrate as soon as possible once CI works properly again.

[devmotion]

Catalyst pins MTK to v9, so CI and releases should be unaffected by breaking changes in MTK.

Similar to how I recently backported support for ForwardDiff@1 and DifferentiationInterface@0.7 to MTK@9, I think it would be good to update/backport compat entries (apart from breaking MTK and SciML releases, of course) to the latest Catalyst version as much as possible.

[TorkelE]

So, in theory that should work. But what may have happened is that some other of the many MTK-adjacent packages have made some form of update which implicitly assumed that MTK v10 has happened, but does not explicitly require MTK v10, and hence is pulled in and causes the error. At least that is my guess, as the CI errors I saw don't really relate to Makie, but had something to do with indexing of MTK structures.

[devmotion]

If these errors are not caused by CairoMakie, then the same errors should show up when running tests on master or the latest release of Catalyst.

SciML packages are unfortunately pretty bad with stable non-breaking releases in my experience, but that's something to be sddressed separately (eg by fixing incorrect SciML compat entries in Catalyst or other packages).

I think nobody should expect an immediate update to MTK@10, given the amount of breaking and fundamental changes and that MTK@11 is supposed to be released soonish. But I think it's annoying for many users and downstream packages if Catalyst is blocking updates of unrelated dependencies such as plotting packages.

[TorkelE]

Didn't think of that this might affect more downstream stuff, that is annoying, definitely. I have started CI, if the error is in MTK indexing, maybe we can try capping SymbolicIndexingInterface. That is the package which does most of the heavy lifting here, could be it.

[pkofod]

Didn't think of that this might affect more downstream stuff, that is annoying, definitely. I have started CI, if the error is in MTK indexing, maybe we can try capping SymbolicIndexingInterface. That is the package which does most of the heavy lifting here, could be it.

thanks!

[jkrumbiegel]

Looks like BifurcationKit's compat also has to be updated as it also has a weakdep on Makie. It already has a version updated to Makie 0.24

[devmotion]

I see similar (same?) test failures in #1338 which reruns CI on master. I haven't checked all logs yet but e.g. https://github.com/SciML/Catalyst.jl/actions/runs/19298250248/job/55185564290?pr=1338 is mainly a problem with the existing tests in Catalyst being somewhat flaky and probably an upstream fix in a newer version of some SciML package - IMO the tests should just check SciMLBase.successful_retcode(sol) instead of explicitly comparing the return code with e.g. SciMLBase.ReturnCode.Success or SciMLBase.ReturnCode.Terminated (as documented: https://github.com/SciML/SciMLBase.jl/blob/ff00df899cad8438f018705c5da7eec0960e909e/src/retcodes.jl#L16-L23).

[TorkelE]

I would have to investiagate closer what is happening, but unless you are talking about a specific case I am not sure exactly what you mean? I.e. in the file which causes the error (1, Core), the only retcode-related test is a

@test SciMLBase.successful_retcode(sol)

and I don't think that is related to the failure here.

[devmotion]

No, the problem I was pointing to was in the Hybrid tests and is caused by

Catalyst.jl/test/simulation_and_solving/hybrid_models.jl

Line 109 in 64c117f

@test sol.retcode == ReturnCode.Terminated

. I opened a PR with a fix: #1339 I haven't checked the other errors yet.

[devmotion]

But regardless of the exact nature of the CI failures - if the same errors are present on the master branch and even more importantly the latest release (I assume they are since it seems there are no implementation changes on master apart from an updated compat entry for Optimization: v15.0.8...master), then there is no harm in merging PRs like this compat update, assuming the relevant plotting tests pass and these versions are actually tested. Users could already run into these issues, so the new release would not make anything worse in that regard; on the contrary, however, users that are currently blocked from e.g. updating CairoMakie would be able to benefit from the latest improvements and bug fixes in CairoMakie.

[pkofod]

Is it the current understanding that none of the failures are related to plotting?

[TorkelE]

No, nothing plotting related

[pkofod]

Okay, so if we relax the related versions that @jkrumbiegel mentioned above it would be possible to make a patch release?

[TorkelE]

Yes, however, we would need to actually sort out the errors so that we do not get CI makred as failing and the docs fails to build. There are some fixes here: #1339, but we are not all the way there yet.

[isaacsas]

Let's see if tests pass with the newer Makie versions now.

[isaacsas]

(If they do I will merge this, check some of the other compat updates that are lingering, and then make a release once we've settled that all.)

[isaacsas]

So seems like BifurcationKit will also need a backported update to the latest Makie version if we want to support it here.

[devmotion]

I prepared a PR to BifurcationKit: bifurcationkit/BifurcationKit.jl#257 Tests in this PR are currently running with the BifurcationKit version in this PR. The test failures of the extension test group on Julia LTS are expected since Pkg supports [sources] only in Julia >= 1.11.

[devmotion]

The BifurcationKit dependency issues seem resolved with my changes, but there are some actual errors remaining in the GraphMakie pipeline, triggered by plot_complexes: https://github.com/SciML/Catalyst.jl/actions/runs/19757606855/job/56612075226?pr=1337#step:6:1822 It seems some actual fixes on the Catalyst side might be needed.

[devmotion]

It seems ef6e150 fixed the previous GraphMakie issue. However, this uncovered that there is another real test error caused by lattice_animation: It seems the (seemingly unofficial?) accesss of .val in https://github.com/pkofod/Catalyst.jl/blob/ef6e15048e7337eb9a82635f27dae5c37025de30/ext/CatalystCairoMakieExtension/cairo_makie_extension_spatial_modelling.jl#L36 does not work anymore in newer Makie versions. I'd assume that the correct (and Makie-version independent) way to achieve the desired dynamic behaviour would be to make the to-be-updated properties an Observable in https://github.com/pkofod/Catalyst.jl/blob/ef6e15048e7337eb9a82635f27dae5c37025de30/ext/CatalystCairoMakieExtension/cairo_makie_extension_spatial_modelling.jl#L27.

[devmotion]

It seems with the backports in BifurcationKit (not officially released yet) and my changes to the plotting extensions and tests, all tests pass successfully and the documentation can be built successfully.

[TorkelE]

If Sam is happy with the new bounds I am happy as well

[isaacsas]

It is fine with me.

[isaacsas]

As I think I’ve said in the past, we should perhaps again consider moving the SI, BK, and Makie extensions to just be separate libraries, to both keep Catalyst leaner, and to avoid them blocking Catalyst releases and testing as we are seeing here.

[TorkelE]

Apparently I cannot commit the suggestions :(

But please feel fre to go ahead and do them, sorry for the hassle