-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
Plotting Catalyst systems no longer works with SciMLBase 2.11 #562
Comments
Plotting still works fine on SciMLBase 2.10. |
No don't overdramatize it, it's just a bug. |
If it helps, I can also confirm the issues occurs if we directly use symbolic maps too, i.e. ...
rn = complete(rn)
p = [rn.α => .1/1000, rn.β => .01]
u0 = [rn.S => 999.0, rn.I => 1.0, rn.R => 0.0]
... |
I don't think that will make a difference. The real issue here is that plot recipes have some spotty coverage since plot functions are called during the tests but not plotting itself. That can be improved because that was setup due to some limitations in Julia v0.6 times, and I think we can run all of the plotting headless now. But reguardless, we need to fix the symbolic indexing in the plot recipe which seems to have regressed with the SymbolicIndexingInterface v0.3 update. But there's a large issue in here which is really that the plot recipe has its own symbolic indexing feature which isn't the actual "symbolic indexing", so it's a bit janky right now. We need to throw out effectively 90% of the plot recipe and just make it use the now existent symbolic indexing features of SciML directly. If you look at the code, the vast majority of it is simply implementing a symbolic indexing feature because that all existed in the plot recipe about a few years before even ModelingToolkit existed, so it just doesn't work like the rest of the system. We should definitely get a hotfix in today ASAP however we can (@AayushSabharwal), but in the longer term the plot recipe needs an overhaul which would make it a lot more stable by using the features that actually exist in the library now, i.e. |
I'll take a look. The plot recipes weren't really touched during the SII PR, just the obvious stuff like removing EDIT: Sheesh this is a mess |
If plotting in tests now works via a headless mode, we can add that to Catalyst at least to catch such issues in the future. |
Yes, we should just add some blank plotting to the tests. One thing at a time though, or accepting PRs 😅 |
Yes absolutely. In no way is the current plotting code any good, it's all technical debt and I'm pretty serious when I say you might be able to straight up delete 90% of the code just by using |
Describe the bug 🐞
I followed the instructions in the tutorial at https://docs.sciml.ai/Overview/stable/. Instead of getting the plot, I got an error.
Expected behavior
The code in the tutorial produces the same plot as shown on the website.
Minimal Reproducible Example 👇⚠️
Error & Stacktrace
Environment (please complete the following information):
using Pkg; Pkg.status()
The text was updated successfully, but these errors were encountered: