-
-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backport #2719 to MTK v8 #2726
Backport #2719 to MTK v8 #2726
Conversation
ODEFunction
expression
Format so tests run |
I reformatted the code (which blows up the diff). It seems only a subset of CI is run on PRs that do not target the master branch - one problem with the existing version bounds for MTK v8 are the breaking changes in SymbolicUtils 1.6.0 (see e.g. https://github.com/SciML/ModelingToolkit.jl/actions/runs/9150353436/job/25155026031#step:6:1243) and the ADTypes changes (and some incorrect version bounds in NonlinearSolve it seems: https://github.com/SciML/ModelingToolkit.jl/actions/runs/9150353436/job/25155025675?pr=2726#step:6:1903). In my package that's not too problematic since I pin these packages anyway but generally that's annoying for users. |
@thazhemadam do you know how to make this run? |
Can we just bound SymbolicUtils? |
Limit SymbolicIndexingInterface as well? |
Why? Most (all?) problems seem to be caused by ADTypes e.g. https://github.com/SciML/ModelingToolkit.jl/actions/runs/9159389647/job/25179664915?pr=2726#step:6:1826 |
The problem here is indeed a package that claimed ADTypes v1 support without ensuring it, namely OptimizationBase. This bug was introduced in OptimizationBase v0.0.6 and only fixed in OptimizationBase v1.0.1, so if we want to amend the general registry we should remove ADTypes v1 compat in OptimizationBase v0.0.6, v0.0.7 and v1.0.0 Cross-ref:
Ping @Vaibhavdixit02 |
Yeah we can do that, I am not sure how it can be changed in the registry though. Can it just be bounded to v1.0.1 here? |
One option is to yank those versions, aka get rid of them completely. That's what I did in JuliaRegistries/General#107391 https://github.com/JuliaRegistries/General?tab=readme-ov-file#when-is-yanking-a-release-appropriate But I'm not sure it is necessary if you tag a v0.0.8 patch release removing the faulty compat entry? |
Let's discuss on the General PR JuliaRegistries/General#107391 |
@gdalle Tests still fail due to ADTypes it seems. |
It's installing OptimizationBase v0.0.7, which I thought I had yanked? |
The package servers can take awhile to get the updated registries. |
r |
The PR backports #2719 to a MTK v8 on a new
backport_8
branch. I can't use MTK v9 yet due to the incompatibility with Catalyst, so #2719 is not sufficient for my project.