-
Notifications
You must be signed in to change notification settings - Fork 19
Automation and pruning for the XIOS handler class #968
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
…ctor Also rework setupAxes to loop over axisIds
c4e1664 to
40eac3f
Compare
09d0fc0 to
282247e
Compare
core/test/XiosCalendar_test.cpp
Outdated
| model.configureTime(); | ||
|
|
||
| // Get the Xios singleton instance and check it's initialized | ||
| // NOTE: The singleton is created during configureTime |
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.
Because Xios::getInstance() is called as part of setting the time, and this runs the code that creates the singleton? Rather than it being explicitly coded as part of Model::configureTime()? If the first is the case, then I think this comment could be better worded.
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.
Good point! Done in 79c6eec.
timspainNERSC
left a comment
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'd like it if the repeated comment in the test were more clearly worded, but it is technically correct as it stands. It just makes it sound like creating the XIOS singleton was specifically coded as part of the function configureTime(), when (as I understand it) that just happens to be the first place that requests the singleton.
Closes #861 Merges into #968 Provides more detail on XIOS, including an introduction, info on the required order of operations, and details on the five XIOS concepts. This PR also makes the file splitting frequency configurable, inlines `setFileSplitFreq`, and removes `setFileSplitFreq` and `getFileSplitFreq` from the interface. Also fixes a related typo.
# Read input file to determine dimensions Fixes #972 Merges into #968 ### 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 Currently, there are differences in how the serial, MPI-parallel, and XIOS configurations read dimensions. For greater consistency, this PR implements a `setDimensionsFromFile` member function for `ModelMetadata`. It gets called in `Model.configureRestarts`. It also gets called in `ParaGridIO.getModelState` (for consistency with the existing implementation). --- # Test Description XIOS tests are updated to account for the fact that dimensions are set in `Model.configureRestarts`. --- # Documentation Impact None --- ### 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
Automation and pruning for the XIOS handler class
Towards #850
Merges into #951
Task List
Change Description
This PR contains refactoring work to (a) automate the XIOS I/O implementation to streamline the user experience and (b) remove member functions that it turns out aren't needed.
Automation
affixModelMetadataintoparseInputFiles,setupDomains,setupAxes, andsetupGrids. CallparseInputFilesin theXiosconstructor and the rest in theParaGridIOconstructor.nCoordsforVERTEXinsetupDomains.nComponentsforDGinsetupDomains.nComponentsforDGSTRESSinsetupDomains.enableXiosby inlining it at the start ofModel.configureTime.Pruning
enableXiosinModel.configureTimeas mentioned above.getClientMPISizeandgetClientMPIRank.setAxisValuesandgetAxisValues.ModelMetadata.setTimebecause it already gets called inModel.configureTime.isInitializedat the end of theXiosconstructor, throwing an error if not.getCalendarTypesince it's hard-coded to"Gregorian".getCalendarTimestepbecause this is given byModelMetadata.stepLength.Test Description
The XIOS handler tests are reworked to account for the automation. This involved some reordering of how things are done.
Documentation Impact
The XIOS docs page has been updated.
Other information
I changed the default
DGCOMPvalue indiscontinuousgalerkin/ModelArrayDetails.cppto be consistent with the default DG degree that gets set in the rootCMakeLists.txt.Pre-Request Checklist