-
Notifications
You must be signed in to change notification settings - Fork 10
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
add wrapper to hide dss internals #293
Comments
I played around with this a little this morning...we can't implement this without adding both ClimaTimesteppers and ODE.jl to the ClimaLSM main project dependencies. This is because, in the implicit model case, we have
I dont mind adding CTS, but ODE.jl really slows down the precompilation time. What do you think? Maybe we can wait on this and for now just add clear documentation that dss! must be used (and why)? |
For now, I will make a PR that adds dss! to all simulations/tutorials and notes that we need to use it. |
Charlie recommended that we could avoid adding ODE.jl by using SciMLBase.jl instead. SciMLBase implements As a note: I noticed that since we now have DiffEqCallbacks in the top-level project, ODE.jl appears in the Manifest under |
Charlie just opened a PR making this fix: #323 |
good idea! |
Is your feature request related to a problem? Please describe.
Currently, when a user is running a mixed implicit/explicit simulation, we require them to construct a ClimaTimeSteppers
ClimaODEFunction
manually. Part of this step involves passing in the dss function (see here). Users may not know what dss is, and we want them to be able to use our model without worrying about it.To solve this, we can create a wrapper function (e.g.
make_ode_function
), which takes in the tendencies, jacobian update function, and Y, and constructs thejac_kwargs
as well as the ClimaODEFunction. This will isolate the usage of ourClimaLSM.dss!
function and make the code more user-friendly.cc @kmdeck
The text was updated successfully, but these errors were encountered: