-
Notifications
You must be signed in to change notification settings - Fork 188
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
Introducing Callback for Simulations #1894
Conversation
Basically a "callback" is a way to run any function that takes a If so, I'm very much onboard with this feature 👍 |
Seems like a good idea but let me make sure I understand it. If I take your example and modify it slightly, change 10 to 1, should it give output everytime step?
|
Yeah... "callback" is an annoyingly obtuse but somehow standard name for this kind of thing https://en.wikipedia.org/wiki/Callback_(computer_programming) https://trixi-framework.github.io/Trixi.jl/stable/callbacks/ We could be rebels and call this something else. Like If |
That's right. |
Yeah, definitely not the clearest name... but if it's standard I guess it's good to keep it. This sounds like it's unambiguously positive though! Nice contribution |
I think I'd like to merge this now, and then make those additional changes later (since the other changes may take some time, and callbacks will be useful in the interim). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@glwagner at some point it might good to think about whether exposing similar interfaces to DiffEq ( e.g. https://diffeq.sciml.ai/dev/features/callback_functions/#The-Callback-Types ) makes sense. Probably for the future. Same could go for run! (e.g. https://diffeq.sciml.ai/dev/basics/integrator/#Initialization-and-Stepping ) too? |
Ah wow, that is epic. I think what's implemented here is more or less analogous to Reading over the features there makes me realize that we probably want to "align" the time-step (somehow, similar to the root finding feature that's provided for DiffEq) for callbacks. We align time-steps for output, but not for callbacks (yet). That would be nice to add. I think we could also add an analog of the "continuous callback" --- to the models, not the simulations --- that's executed during Initialization and finalization might be good wishlist features too for both. |
This PR implements
Callback
, designed to be used withSimulation
.Illustration:
we get
We also support passing an iterable of callbacks to
run!
:just in case someone wants to do that...
I think we could also redesign
TimeStepWizard
to be a special kind ofCallback
, and nuke theprogress
property.What do others think? Are they ok with this big change to the API?
cc @navidcy @ali-ramadhan @francispoulin