Allow TimeStepper's StopTime to be optional #400
Open
andrewdnolan wants to merge 4 commits intoE3SM-Project:developfrom
Open
Allow TimeStepper's StopTime to be optional #400andrewdnolan wants to merge 4 commits intoE3SM-Project:developfrom
TimeStepper's StopTime to be optional #400andrewdnolan wants to merge 4 commits intoE3SM-Project:developfrom
Conversation
22 tasks
And narrow the scope of the function to only do module initalization restart / initial condition reading/checking is moved back into `ocnInit` proper.
f375136 to
2802f6d
Compare
Author
|
Documentation: Updated the developers guide to reflect the new argument order and detail the Local build of the documentation can be found here:
|
TimeStepper's StopTime to optional TimeStepper's StopTime to be optional
Author
Testing:CTest unit tests
Polaris
|
xylar
reviewed
May 7, 2026
|
|
||
| In standalone simulations, `TimeStepper::init1();` will read the StartTime from | ||
| the configuration file. For coupled simulations the StartTime is provided by | ||
| coupler, overriding the value defined in the configuration file. |
There was a problem hiding this comment.
Suggested change
| coupler, overriding the value defined in the configuration file. | |
| the coupler, overriding the value defined in the configuration file. |
xylar
approved these changes
May 7, 2026
xylar
left a comment
There was a problem hiding this comment.
I don't feel super expert in Omega time stepping but this approach looks clean to me. I'm approving mostly on the basis of it seeming like a sensible and extensible design.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR enables
StopTimeto be optional forTimeStepper's. In coupled E3SM simulations components do not know the stop time (coupler is responsible for calling finalize).The optional
StopTimewas enabled by reordering the arguments toTimeStepper::createso thatStopTimewas the last argument, which is a requirement ofstd::optional.An overload to
TimeStepper::init1is added that accepts a structure (TimeInitParams), which will be how the coupler will provides theStartTimeinstead of it being read from the config file.I've also added an overload to
initOmegaModuleswhich accepts thisTimeInitParamsstructure, which will be used in the coupled version ofocnInit. In refactoringinitOmegaModulesto be suitable for both standalone and coupled configuration, I moved a bunch of code not related to init'ing modules frominitOmegaModulesintoocnInit. This code was primarily related to reading the restart and initial condition IOStreams, which will need different handling in coupled mode.While this PR adds an overload to
initOmegaModulesthat acceptsTimeInitParams, I have intentionally not added an analogous overload toocnInit. Exactly what we passocnInitin a coupled simulation is still being scoped out and how to pass that (e.g. all as separate arguments or as struct(s)) is yet to be determined. A follow on PR that adds coupled overloads forocnInit,ocnRun, andocnFinalizewill come later. Given that the work enclosed in this PR is self contained, I figured it would make sense to get reviewed and merged while we figure out more details for the coupled driver.Checklist
Testingwith the following:have been run on and indicate that are all passing.
has passed, using the Polaris
e3sm_submodules/Omegabaseline-pfor both the baseline (Polarise3sm_submodules/Omega) and the PR build