-
Notifications
You must be signed in to change notification settings - Fork 19
Introduce static TimePartition
struct for Iterator
#932
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
Conversation
testXiosCalendar_MPI1 | ||
PRIVATE "${MODEL_INCLUDE_DIR}" "${XIOS_INCLUDE_LIST}" "${ModulesRoot}/StructureModule" | ||
PRIVATE | ||
${PHYSICS_INCLUDE_DIRS} |
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.
I had to add these includes to each of the XIOS tests to avoid linking errors when including Model.hpp
.
|
||
// Create a Model and configure it so that options are parsed | ||
Model model(MPI_COMM_WORLD); | ||
model.configureTime(); // TODO: Use Model.configure to parse restart files this way, too? |
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.
To address this TODO (and the similar ones in the other tests), we'll need to align the config option used for restart files with and without XIOS. It would make sense to me to do so, rather than having separate XIOS-specific options. We want to make the user experience as similar to the non-XIOS case as possible.
I need the model start and stop times for issue #926. I wrote some code yesterday to move the time logic to |
Okay great, thanks @timspainNERSC. |
See #936 |
Closing in favour of #936. |
# Add time limits to `ModelMetadata` Fixes #932 (partially) --- # Change Description Remove the time limit calculations (such as they are) from `Iterator` to `ModelMetadata`. Also, store the time limits in `ModelMetadata` and output them to the restart file through `CommonRestartMetadata`. As an added advantage, these values are now actually written to the restart file. --- # Test Description Ran the standard model configuration by hand to make sure the run length was still correct. The integration tests in the CI suite will provide automated testing. --- # Documentation Impact None: purely an internal change. --- # Other Details As it stands, this may currently break XIOS, as no configuration related to start time, stop time or time stepping is now passed to XIOS.
# Use model config in XIOS Fixes #925 Follows #936 Second attempt following #932 ~~Merges into #913~~ ### Task List - [x] Defined the tests that specify a complete and functioning change (*It may help to create a [design specification & test specification](../../../wiki/Specification-Template)*) - [x] Implemented the source code change that satisfies the tests - [x] Documented the feature by providing worked example - [x] Updated the README or other documentation - [x] Completed the pre-Request checklist below --- # Change Description This PR reuses the time metadata set as part of the config set in the XIOS handler. --- # Test Description For the purposes of testing, a separate `Model.configureTime` member function is created to avoid having to configure *everything*. I also needed to get the build system to create partition metadata files for 1 and 3 MPI ranks for the XIOS tests and to extend the configuration for all of them. --- # Documentation Impact The XIOS doc pages are updated to reflect these changes. --- ### Pre-Request Checklist - [x] The requirements of this pull request are fully captured in an issue or design specification and are linked and summarised in the description of this PR - [x] No new warnings are generated - [x] The documentation has been updated (or an issue has been created to track the corresponding change) - [x] Methods and Tests are commented such that they can be understood without having to obtain additional context - [x] This PR/Issue is labelled as a bug/feature/enhancement/breaking change - [x] This change conforms to the conventions described in the README
Introduce static
TimePartition
struct forIterator
Fixes #925
Task List
Change Description
This PR follows the recommendation in #925 to reduce duplication of configuration code by introducing a
TimePartition
struct and a static instance of it associated with theIterator
class. This gets set up duringModel.configure
and may be accessed from other classes via theIterator::getTimePartition
interface.Test Description
The XIOS tests are updated to be configured by
Model
, so all of them now create an instance of this at the start. However, they currently use a reducedconfigureTime
because setting up restart and partition files seems unnecessary for most of them.Documentation Impact
The XIOS doc pages are updated to reflect these changes.
Pre-Request Checklist