Drop XiosOutput section#1063
Drop XiosOutput section#1063joewallwork wants to merge 24 commits intoissue1056_xios-read-all-inputsfrom
XiosOutput section#1063Conversation
joewallwork
added
enhancement
| // FIXME: We need to write out u and v to get a perfect restart | ||
| // xiosHandler.setPrognosticFieldType(uName, ModelArray::Type::H); | ||
| // xiosHandler.setPrognosticFieldType(vName, ModelArray::Type::H); |
There was a problem hiding this comment.
Note to self to complete this before marking the PR as ready for review.
There was a problem hiding this comment.
The issue I was having here has to do with the u and v fields being expected to be type Model::Array::U and Model::Array::V, which I hadn't accounted for. Working on a fix.
@timspainNERSC: It seems to me that these types are duplicates of Model::Array::H. Is that the case or are there distinguishing factors?
There was a problem hiding this comment.
In the current set up, these are indeed duplicates of HField. The design exists to be able to extend nextSIM-DG with other dynamics cores (such as the Lagrangian) where the u and/or v arrays might have different dimensions from the h arrays.
There was a problem hiding this comment.
Okay good to know, thanks. I think it would be best to do this separately from this PR. I opened issue #1066 for tackling this and added a reference in the code.
| const bool spherical = true; // TODO: Determine this | ||
| if (spherical) { | ||
| xiosHandler.setPrognosticFieldType(latitudeName, ModelArray::Type::H); | ||
| xiosHandler.setPrognosticFieldType(longitudeName, ModelArray::Type::H); | ||
| } else { | ||
| xiosHandler.setPrognosticFieldType(xName, ModelArray::Type::H); | ||
| xiosHandler.setPrognosticFieldType(yName, ModelArray::Type::H); | ||
| } |
There was a problem hiding this comment.
Question for reviewer: I see that there's an enum for this in ParametricMesh but not in ParametricGrid. It feels like it would make sense to align this - would that be agreeable? The difficulty I'm having is that I want to determine the coordinate system upon constructing ParametricGrid, when there likely isn't a ModelState I can query.
There was a problem hiding this comment.
The sphericity versus planarity of the coordinate system is currently determined by the restart file. A solution would be to peek inside the restart file to check if there is a longitude field (=> spherical coordinates) and if there is not, an x field, the lack of which would be an error. The ModelMetadata class does something similar to get the dimensions around ModelMetadata.cpp:200.
There was a problem hiding this comment.
Thanks for the suggestion. Done in 1f33b5e.
There was a problem hiding this comment.
Also throw the recommended error in 88a9d74.
| bool configuredRestarts = false; // Guard to avoid repeated configuration | ||
| bool configuredTime = false; // Guard to avoid repeated configuration |
There was a problem hiding this comment.
Is XIOS configuration not idempotent? The rest of the model can be configured multiple times without ill effect.
There was a problem hiding this comment.
Unfortunately not. Parts of it could be made so if we wanted that but the context is problematic because it cannot be overwritten.
timspainNERSC
left a comment
timspainNERSC
left a comment
There was a problem hiding this comment.
This all seems in order.
2 participants
Drop
XiosOutputsectionFixes #1056
Merges into #1057
Task List
Change Description
This PR allows us to drop the temporary
XiosOutputconfig section. In order to achieve this, I needed to ensure that each module that defines agetStatePrognosticmember function also sets XIOS field types appropriately.Test Description
I noticed that some fields were slipping through the cracks in the XIOS tests so I made them more robust. That is, I added checks that the expected fields are all found in any input files.
I also dropped
damagefrom the test input file to check that it gets set up automatically during model configuration, as expected.Documentation Impact
The XIOS docs page has been updated accordingly.