consider moving some extensions to separate libraries

[isaacsas]

Issues like https://github.com/SciML/Catalyst.jl/actions/runs/10549534673/job/29224675728?pr=961 are due to some of the extension libraries we depend on not updating to newer (breaking) releases of packages. In this case I think Homotopy continuation, see JuliaHomotopyContinuation/HomotopyContinuation.jl#581, still being on Arblib .8 instead of 1.X is going to block us from updating DynamicPolynomials, but ultimately Symbolics too (which now also requires DynamicPolynmials 6). Note that this isn't an issue with Arblib just recently making a breaking release, v1.0 came out 10 months ago.

It seems like we might want to move such extensions to be separate libraries (i.e. CatalystHomotopyContinuation or such), which would at least allow Catalyst to get updated for everyone that doesn't need to load HomotopyContinuation.

[PBrdng]

Hi, the problem here is that using Arblib 1.0 or newer causes some of the crucial tests in HomotopyContinuation to fail. I'm on it, but I don't know how long it will take to fix this. Sorry!!

[isaacsas]

No worries, and thanks for looking into it!

[ChrisRackauckas]

We can probably relax the DynamicPolynomials. The sad thing is then you'll have a Pkg dependency with DynamicPolynomials v5

[isaacsas]

This may all be indicative that we should split off a Catalyst core at some point.

[isaacsas]

I think I'm going to go ahead with this change in the next few weeks as part of Catalyst 15 unless anyone objects. All the tests that are currently broken seem to be extension-related, and we've had enough cases of extensions holding back dependency updates (and other issues), that I think it makes sense to split them off from core Catalyst. The idea is we'd then have CatalystBifurcationKit, CatalystStructuralIdentifiability, CatalystHomotopyContinuation, etc, which users would explicitly import to get the current extension functionality.

[isaacsas]

The only question is how to handle extensions in the docs with such a change. Perhaps we should just setup the extension-related documentation with the extension libraries, and then just link to it in the Catalyst docs. This would allow us to avoid having Catalyst docs also depend on the extensions (otherwise we will simply move the problem from Catalyst to Catalyst's docs).

[TorkelE]

I'm not keen on doing this right now. Maybe later when I can look at it properly. Last few days have been quite hectic, so haven't been able to look at the tests, but will try to help as soon as possible.

[isaacsas]

What is the benefit to keeping extensions though? As far as I can tell there is none. They aren't really easier on a user, i.e. saying

using Catalyst, BifurcationKit

is basically the same as saying

using Catalyst, CatalystBifurcationKit

and it seems like it would be a big benefit to us in being able to decouple updates to Catalyst from updates to the extension libraries.

[isaacsas]

In fact, we can have CatalystBifurcationKit reexport Catalyst, so a user would only need to say

using CatalystBifuractionKit

[isaacsas]

Also, keep in mind we can setup downstream tests, which we should probably be doing anyways for certain other packages, to check with each PR if the extension library breaks. So we'll still see immediately if updates to core Catalyst break something in an extension ilbrary (but won't be held back from merging those updates to Catalyst and making Catalyst releases).

[TorkelE]

It seems potentially a lot more overhead in that we have to manage several additional packages. And we'd want to have those as downstream tests anyway, so I'm not sure we'd actually gain anything?

I'm not saying that I'm categorically against it. However, I'd actually want to go through what the implications would be when I think of it properly. Maybe we should also discuss through what current and future features of Catalyst we'd want in base/extensions/other packages. Especially as we are reaching a point where things are getting quite settled (baring more changes in MTK).

If we want v15 asap, maybe just mark broken tests as broken, that should be the fastest thing (and also correct, if things are broken).

Edit: saw your comment about downstream tests, you mean like having a separate test set and letting it fail?

[isaacsas]

Making a release with all the extensions broken seems like a bad idea when they are coupled into Catalyst like this. (Hence why decoupling them would be nice, then they could remain on V14 until someone has the time to fix them.)

[isaacsas]

And yes, I mean that we can test each extension library as a downstream test, like MTK does for Catalyst, as part of an additional CI script. When it fails we'd know a PR broke an extension and it needs to be fixed, but that wouldn't prevent us from making Catalyst releases.

[TorkelE]

Moving them out still would not change whether they are broken or not? Short term the whether the functionality does/does not work won't really change. Long term an argument can made, however, all problems (and none until now have caused any major delays) have been related to MTK v9 and follow-up updates. Hopefully, that should calm down (although admittedly whether that is the case is hard to tell).

Either way, I am not directly convinced that separate packages are advantageous, and if we do change it I would actually like to think it through and not rush it.

[TorkelE]

I will try help figure out what is going on as well. The MTK Bifurcation tests do seem to work, so its not fundamentally broken. Ours are more extensive though

[TorkelE]

In the case we go down the separate package routes, maybe it would make more sense to simply have a single package, CatalystExtended, which contains minor and tangential functionality like this? Would enable us to keep package numbers down (although we might also have network analysis and spatial modelling as separate ones...). Just a quick idea I got while taking a shower though, but figured I'd add it to the discussion.

[isaacsas]

Keep in mind we don't need separate repos for them. They can all live in sub-folders of a lib folder in Catalyst. So it would all still be in one place.

edit: Or at least the more minimal extension libraries would make sense to just house here. More involved ones could be their own repos.

[ChrisRackauckas]

Either way, I am not directly convinced that separate packages are advantageous, and if we do change it I would actually like to think it through and not rush it.

It's really disadvantageous and using extensions is a much better idea if we can get away with it. For many of these cases, they are truly extensions, so I'm not sure why we would take the overhead burden of splitting to new packages. Right now the big issue is how to not split to more packages, not how to get more 😅 . See JuliaLang/julia#55516 for details.

[TorkelE]

Keep in mind we don't need separate repos for them. They can all live in sub-folders of a lib folder in Catalyst. So it would all still be in one place.

That make sense. If we do split extensions of that sounds like the way to do it. splitting minor stuff

I (think I) have fixed the BK and HC tests. The extensions were fine actually, but some tests seemed to depend on parameter ordering/MTK internals. This is fixed now (although especially the BK test can probably be rewritten better, and maybe should be removed).

The SI problem I have gotten somewhere with, but not fixed. Basically, we introduce array variables in the MTK system we generate from a ReactionSystem (and which we test identifiability on). Not using array variables makes everything be fine. I will try to figure out why array variables and SI don't work (or alternatively, rewrite our code not to depend on it).

Right now I don't see how splitting of the extensions fixes things, Identifiability will be broken wherever we put the code. In #1070 I commented out the tests for now. It might also make sense to move extensions to a separate test set which is run independently from the main CI test (if we want to allow it to be marked as failed without affecting the other tests). I am not fully sure how to set that up though (but could check, how other packages do it).

[PBrdng]

Hi guys, I'm sorry that my inactivity in updating HC.jl has caused such a big discussion. The reason for the delay was that at the same time I became the only maintainer for the package (the other person quit) and started a professor position. This was too much to handle, apparently. It's not an excuse but an explanation.

That being said, I promise to be much faster in reacting to issues in the near future. Hopefully, this info can help you in your discussion, at least when it comes to HC.jl.

In fact, I offer to help fix broken tests that depend on HC. Just send me an email!

[TorkelE]

No worries @PBrdng . In this case the error was entirely independent of HC (it had to do with how I had written the tests). Part of the discussion is also on how to handle non-core features (i.e. move them to sub packages), and not really a worry about dependencies.

HC is an awesome package, really happy we have something like that in Julia. We discussed to move the Catalyst integration so that HC can be applied directly to all ModelingToolkit NonlinearSystems. Then people who use MTK can use HC without declaring the HC Systems separately. And then Catalyst can just wrap all this. Which reminds me that I promised to look at that and totally forgot about it...

[isaacsas]

@PBrdng as @TorkelE said this isn’t meant to be a HC criticism. It is a great package and works very nicely with Catalyst. I also very much appreciated your quick fix in updating the dependencies!

This issue is meant more as a place to figure out the right way to include such couplings in Catalyst (ie via extensions or independent libraries). Moving them to separate libraries has a number of benefits as I’ve mentioned above.

@ChrisRackauckas that Julialang issue would certainly help with fixing the issues I mention, but is it likely to appear on any short term timescale? Beyond that are there other reasons for not switching? I certainly agree maintaining 50 libraries as you now have in OrdinaryDiffEq is a mess. But here it would be three, each with essentially one non-Catalyst dependency (which would no longer be a Catalyst dependency so we’d just be moving the work from updating Catalyst for new releases to them, and better isolating them if such work takes a while to accomplish).

[ChrisRackauckas]

I would expect it in v1.13.

[isaacsas]

Just to return to this issue; the SI extension is now causing us to be completely unable to even run tests:

https://github.com/SciML/Catalyst.jl/actions/runs/11239742228/job/31248152175?pr=1053#step:6:1197

So we are completely broken currently due to having it as an extension that needs to be complied as part of the normal test CI process (instead of having it as a separate library that could be tested in a downstream CI script).

[isaacsas]

I'm going to remove it completely on master until this Julia crashing issue gets fixed in SI as otherwise we can't even run CI (right now it was removed from CI but not removed from Project.toml and such).