Skip to content
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

[FRAMEWORK] Add a new name_in_output registry attribute to variables #23

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented May 24, 2022

This can be used to set a different name in output streams than the name in code or the unique name required for input.

This merge also adds corresponding outputFieldName and outputConstituentNames to each field. These new attributes will be used to allow different names in output than in the code. Currently, the output name must be unique, but this causes trouble in time mean output, particularly for coordinate variables.

@xylar xylar force-pushed the mpas-framework/add-name-in-output-attribute branch from 91dfcd1 to 55b8ad6 Compare May 26, 2022 02:38
@xylar xylar force-pushed the mpas-framework/add-name-in-output-attribute branch 2 times, most recently from dc620a3 to 60c17dc Compare August 5, 2022 11:36
xylar added 10 commits August 6, 2022 17:03
These new attributes will be used to allow different names in
ouput than in the code.  Currently, the output name must be
unique, but this causes trouble in time mean output, particularly
for coordinate variables.
This can be used to set a different name in output streams than
the name in code or the unique name required for input.
@xylar xylar changed the base branch from master to alternate August 6, 2022 15:10
@xylar xylar changed the base branch from alternate to master August 6, 2022 15:10
@xylar xylar force-pushed the mpas-framework/add-name-in-output-attribute branch from 60c17dc to 3781e1d Compare August 6, 2022 15:11
@xylar xylar requested review from akturner and mark-petersen August 6, 2022 15:11
@xylar
Copy link
Collaborator Author

xylar commented Aug 6, 2022

@mark-petersen and @akturner this is the groundwork for supporting Time and Time_bnds coordinates in MPAS-Ocean and MPAS-Seaice. I need the ability to have a different name in output files than in the code because of time-series-stats. We don't want timeMonthly_avg_Time, etc., because these are not acceptable coordinate names (and they're also confusing).

To accomplish this, I have simply added an outputFieldName attribute and outputConsistentNames array to each variable or var_array. The default value is the same as fieldName and constituentName, respectively.

The first level of testing I would request from each of you is to run a standalone test of your choice to make sure things don't break. @akturner, if there is a test you have that produces timeSeriesStatsDailyOutput in standalone, that would be especially awesome! Then, I would ask you to suggest a test or 2 for me to run in E3SM to sanity check things there. (I've been running standalone MPAS-Ocean via compass so far, including the daily_output test that is pretty thorough).

Finally, if you actually want to test the name_in_ouput attribute in the registry, you can pick a variable of your choice (one that doesn't go into restart files is best, since renaming those variables will lead to problems if you restart) and add a name_in_output attribute in the registry to give it a new name. In the streams file, give the name in code as usual -- this is not easy to avoid because the name in code is unique, whereas the name in output won't typically be, because that's the whole point of this work.

I successfully gave daysSinceStartOfSim, atmospherePressure and zMid different output names in MPAS-Ocean.

I have also tested this with further changes in #19 that implement the Time and Time_bnds coordinates in MPAS-Ocean.

@xylar xylar requested a review from cbegeman August 6, 2022 15:39
@xylar
Copy link
Collaborator Author

xylar commented Aug 6, 2022

@cbegeman, I've added you as well. Whatever testing and/or looking over the code you'd like to do would be most appreciated.

@mark-petersen
Copy link

I compiled and tested with gnu and intel on badger, both with debug. I added

+++ b/components/mpas-ocean/src/Registry.xml
+                        description="horizontal velocity, normal component to an edge" name_in_output="normalVelocityChanged"
+                        description="layer thickness" name_in_output="layerThicknessChanged"
+                        description="Time since simulationStartTime, for plotting" name_in_output = "daysSinceStartOfSimChanged"
+++ b/components/mpas-ocean/src/tracer_groups/Registry_activeTracers.xml
+                        description="potential temperature" name_in_output="temperatureChanged"

and those did indeed show up in the output as those names. As Xylar said, this breaks restart files. If I only add name_in_output to non-restart variables, it passes the nightly test suite.

Copy link
Collaborator

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks great to me! I tested changing an output name with mpas-o standalone on chrysalis with the intel-mpi compiler.

Copy link

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Miraculously, I was able to change outputFieldName directly inside the code be dealing with the statePool structure directly. I first tried this:

statePool % layerThickness % outputFieldName = 'layerThicknessChanged' 

but that was how you managed variables before pools. What I actually needed to do was this (with print statements added)

     print *, statePool % iteration_head % key
     print *, statePool % iteration_head % keyLen
     print *, statePool % iteration_head % data % r2a % fieldName
     statePool % iteration_head % data % r2a % outputFieldName = 'normalVelocityChanged'
     print *, statePool % iteration_head % data % r2a % outputFieldName

which feels like running electrical wires outside of the conduit. We don't ever do this because you need to know where your variable is in the linked list, and what type it is (r2a for 2D real array). After all that, the new name does appear in the output!

@xylar
Copy link
Collaborator Author

xylar commented Aug 9, 2022

@mark-petersen, the "right' way to do what you're trying to do would be first to pick a field that isn't in a restart file as you noted. I will use zMid as my example. Then, you need to get the field, not just the array, from the pool:

type (field2DReal), pointer :: zMidField

...

call mpas_pool_get_field(diagnosticsPool, 'zMid', zMidField)
zMidField % outputFieldName = "zMidOutput"

I haven't tested this exact example but you can see similar things in my ocn/add-cf-time branch, e.g.:
https://github.com/E3SM-Ocean-Discussion/E3SM/pull/19/files#diff-b3ca7b1e079038a274cd2a9cd26a90dc4fa3f3f6dae740dbcbb560803256bee4R1264

@mark-petersen
Copy link

Yes, good point. Using mpas_pool_get_field worked too.

Copy link
Collaborator

@akturner akturner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built MPAS-Seaice and run standard standalone tests, including with the daily time series stats AM. Verified that output name can be changed with the name_in_output attribute in standard output stream.

@xylar
Copy link
Collaborator Author

xylar commented Aug 10, 2022

Moved to E3SM-Project#5120

@xylar xylar closed this Aug 10, 2022
xylar pushed a commit that referenced this pull request Nov 8, 2023
…w-icepack

Corrects use of snow flags and adds flag consistency checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants