Modify MPAS-Ocean Registry files to have CF compliant units #24

darincomeau · 2022-05-24T16:03:55Z

This modifies MPAS-Ocean Registry files to have CF compliant units, using cfchecker to find errors.

[WIP]

Modify the namelist variables to allow up to 15 EAM output tapes. [BFB] - Bit-For-Bit [NML] - Namelist Changing

Add CF compliant units to registry Change non-CF compliant units in registry to CF compliant ones Reformat Registry for clarity Moved incremental remapping fields to separate registry file

darincomeau · 2022-05-24T16:24:43Z

Some remaining errors from using cfchecker on timeSeriesStatsDaily test case:

units="NA" or "nondimensional". I believe some of these should be "1", while others should have no units attribute, so this probably needs to be looked at individually.
units with PSU.
xtime_startDaily, xtime_endDaily, and timeDaily_counter have units="unitless", but these do not appear in mpas-ocean Registry files.

rljacob · 2022-05-24T17:43:12Z

In EAM and ELM, we gave dimensionless variables a unit of "1" which is CF compliant. That seemed better than no "units" attribute at all. Not sure if we want an E3SM-wide standard for that or not.

xylar · 2022-05-25T00:23:18Z

In EAM and ELM, we gave dimensionless variables a unit of "1" which is CF compliant. That seemed better than no "units" attribute at all. Not sure if we want an E3SM-wide standard for that or not.

As @czender explained it to us, there are variables (like slope or other dimensionless ratios) where a unit of "1" makes sense. But there are lots of variables where no unit (even "1") would make sense, like an integer or logical field. So I would think places with units="NA" should not have a units attribute and ones with nondimentional or unitless might go either way. I agree with @darincomeau that they really need to be dealt with individually.

xylar · 2022-05-25T00:24:06Z

xtime_startDaily, xtime_endDaily, and timeDaily_counter have units="unitless", but these do not appear in mpas-ocean Registry files.

@darincomeau, I can deal with these as part of my work on the Time coordinate if you like.

darincomeau · 2022-05-25T00:39:12Z

Thanks for the comments @xylar . There are only a handful of units="NA" or "nondimensional", but I was indiscriminate about wiping out all units="unitless" in this branch, so those may need more careful consideration.

For the time coordinate units, I haven't been able to find it from a cursory grep of framework yet. So either you or I could do it, but either way seems like it would be a separate branch from this one with strictly ocn Registry changes.

xylar · 2022-05-25T01:51:31Z

For the time coordinate units, I haven't been able to find it from a cursory grep of framework yet.

I think you'll find it in the time series stats analysis member in the ocean core. I believe it must be defined here:
https://github.com/E3SM-Project/E3SM/blob/master/components/mpas-ocean/src/analysis_members/Registry_time_series_stats.xml#L10-L31
We could remove units="unitless" from here and see what happens at least...

darincomeau · 2022-05-25T02:42:18Z

Thanks @xylar for the pointer, that was indeed the place. Now down to just 5 NA/nondimensional errors...

darincomeau · 2022-05-25T19:26:53Z

Discussion items from this morning's hackathon meeting:

PSU has been removed from all units attributes in the files changed so far, replaced to 1.e-3. Descriptions have not been updated, with the exception of the salinity tracer, where "grams salt per kilogram seawater" was added. Question for the ocean developers - is it appropriate/desired to add PSU to the description attributes?
Regarding unit="unitless" removal:

many of the instances removed so far from Registries are nml records. They have no impact on CF compliance, but offer no useful information; I can add them back in if there's a preference to, but to me it seems cleaner to keep them out.
there's at least one example I saw where units="1" would be appropriate (something like maxCFL). I'll go through more carefully and try to compile a list of where I've added this 1 unit designation as part of the review.

darincomeau · 2022-05-25T22:50:42Z

Question: Should dimensions that are clearly integers (e.g. nCells) have the units="1" attribute? Or no units attribute?

czender · 2022-05-25T23:25:40Z

@darincomeau Attributes can only be attached to netCDF variables, not dimensions, so I do not understand the context of your question. If there is somewhere a (coordinate) variable named nCells, with underlying dimension nCells?

xylar · 2022-05-25T23:27:18Z

A agree with @czender. If the question refers to an index array instead (e.g. cellsOnCell), it should definitely not have units. I can't think of any integer or logical array that would usefully have units.

darincomeau · 2022-05-26T00:18:10Z

@czender @xylar thanks for the comments. It seems like these shouldn't have been introduced at all, and this branch is clearing them. I don't see how to link file changes here, but if you look at the Files Changed in this PR, and open up Registry.xml, you'll see at the top lines where I've removed units="unitless" previously tied to dimensions.
So if I understand correctly this is a desired change, and these should be removed.

xylar · 2022-05-26T02:24:01Z

components/mpas-ocean/src/Registry.xml

-		<dim name="nCells" units="unitless"
+		<dim name="nCells"
 			 description="The number of polygons in the primary grid."


@darincomeau, yep, this is definitely a desired change. To comment, you want to go to Files changed and use the blue plus sign to highlight one or more lines. Then, type your comment.

There isn't another way to link to the diff as far as I know. You can link to the file itself.

Collaborators have requested that these fields always be included in simulations with ice-shelf cavities so they can study the relationship between melt rates, thermal driving and friction velocity in the sub-ice-shelf boundary layer across many models. This output would also likely be of interest to us, particularly as we explore new vertical coordinates in ice-shelf cavities.

Add 5 ice-shelf BL fields to MPAS-Ocean monthly output Collaborators have requested that these fields always be included in simulations with ice-shelf cavities so they can study the relationship between melt rates, thermal driving and friction velocity in the sub-ice-shelf boundary layer across many models. This output would also likely be of interest to us, particularly as we explore new vertical coordinates in ice-shelf cavities. [BFB]

xylar · 2022-06-16T07:10:20Z

@darincomeau, could you rebase when you get a chance to handle the 2 conflicting files (and make sure no new non-CF units were added in the meantime)?

xylar · 2022-06-16T07:15:04Z

@vanroekel, @milenaveneziani, @mark-petersen could the 3 of you have a look at these changes? Let us know if there are questions. The gist is that CF compliance requires that

variables not have a units attribute if they are unitless (i.e. no units='unitless', units='NA', etc.)
variables should have units=1 if they are dimensionless (e.g. real-valued coefficients)
we cannot use latex symbols like { and } in the units
we cannot use units='PSU' for salinity, we need to use units=1e-3, though we can state that the units are grams per kilogram in the description (used as the long_name attribute in output files).

@darincomeau, please edit if I missed anything.

Everyone, please feel free to ping others to have a look.

vanroekel

These changes seem consistent with the CF compliance defined in the comment. This is a great piece of work, lots of very careful combing through registry.

vanroekel · 2022-06-16T14:47:54Z

@xylar or @darincomeau a stupid question -- does the Registry name, units, etc... populate the netcdf output metadata? I guess that makes sense, but I had originally thought that was just for the users guide.

xylar · 2022-06-16T15:17:34Z

@vanroekel, yes, many of those things end up as attributes in the NetCDF output. Since we aren't doing a good job at keeping the user's guide updated, that is the primary thing they do at this point.

darincomeau · 2022-06-16T16:44:51Z

@xylar yes I'll do a rebase, and thanks for providing the summary. Some notes/comments:

The most recent commit is one to look over carefully, as I made some decisions on what was unit="1" or no units attribute at all
Note this PR removes units="unitless" attributes from many nml and dim entries. This may not strictly be needed for CF compliance, and may be more classified as cleanup, hope it's ok with everyone for the changes to be all together.
Using cfchecker on the compass daily_output_test, where I replaced the output variables to match E3SM's timeSeriesStatsMonthly output, I get zero errors on variable units.
However, I've stumbled upon the following units still in Registry files that are not CF compliant, so there's almost surely others:
-- units=""
-- units= petawatts, or Sv
Also, this PR does not touch any of the following Registry files in tracer_groups (BGC), and I'll make an issue when I move this PR over to keep track of this:
-- Registry_CFC
-- Registry_DMS
-- Registry_MacroMolecules
-- Registry_ecosys
-- Registry_idealAge

xylar · 2022-06-16T17:11:09Z

Note this PR removes units="unitless" attributes from many nml and dim entries. This may not strictly be needed for CF compliance, and may be more classified as cleanup, hope it's ok with everyone for the changes to be all together.

I agree, this clean-up belongs as part of the PR. These units aren't useful and it's a lot easier to clean them up everywhere, not just in variables.

units=""

Are these ones where you can just remove them (like `units="NA")?

units= petawatts, or Sv

Do you know what CF wants us to do for these? units="10^15 watts" and units="10^6 m^3 s^-1", respectively?

Allow up to 15 EAM history tapes The "SPECIAL" v2 production runs require more than 10 EAM history output tapes (https://acme-climate.atlassian.net/wiki/spaces/EWCG/ pages/1850376218/E3SMv2+history+variable+namelists+for+production+ runs), exceeding the current limit (10 tapes). This PR includes changes to both the code and namelist variables and increases the limit to 15 tapes. We should merge this to both maint-2.0 and master, but when I issued the PR for merging to maint-2.0 (#4891), lots of unrelated commits were included (We only need the two commits in the present PR). I am not sure how to fix the PR to maint-2.0, so closed it for now. The tests are successful and located at /lcrc/group/e3sm/ac.qtang/ E3SMv2/test-output-maint-2.0.v2.NARRM.amip/tests. * tangq/atm/increase_eam_tapes: Change code to support up to 15 EAM tapes Increase the max number of EAM history tapes to 15

czender · 2022-06-16T17:29:36Z

Sverdrup (but not Sv) is well-defined by UDUnits and thus CF:

zender@spectral:~$ udunits2
You have: Sverdrup
You want: m3 s-1
    1 Sverdrup = 1e+06 (m3 s-1)
    x/(m3 s-1) = 1e+06*(x/Sverdrup)

petawatts is also well-defined by UDUnits and CF, so I do not know what the problem with the checker is, but it is acceptable to use "petawatts" as a unit.

You have: petawatts
You want: watt
    1 petawatts = 1e+15 watt
    x/watt = 1e+15*(x/petawatts)

Cleanup of sea ice registry Cleanup of mpas-seaice Registry files, including: * Add CF compliant units to Registry * Change non-CF compliant units in Registry to CF compliant ones * Reformat Registry for clarity * Moved incremental remapping fields to separate Registry file [BFB]

darincomeau · 2022-06-16T18:40:25Z

Thanks @xylar , @czender . I haven't actually run any variables with units "petawatts" through the checker yet, I only tried searching for that and Sv in the CF Standard Names table and didn't see them. "Sv" appears only twice Registry_transect_transport, so I'll change that to Sverdrup, and leave petawatts alone (only appears 4 times in Registry_meridional_heat_transport.

Regarding units="", there's actually only a handful of them as well, and looks like most should be 1, so I'll do those in a separate commit now.

darincomeau · 2022-06-16T18:45:45Z

...and just found some units="none" and units="None"

…iate

…Registry files

darincomeau · 2022-06-16T19:44:45Z

welp, i messed up the rebase

Edit: Actually I think I did it correctly, and the unintended changes showing up here are just very recent E3SM changes. If I go to open a PR from this branch into E3SM, only the intended commits/file-changes show up.

Anyways, apart from the BGC Registry files, I've now changed everything I'm aware of and intended to do for this PR.

xylar · 2022-06-16T20:02:51Z

Yeah, I can update E3SM-Ocean-Discussion/E3SM/master again but that's not necessary. Go ahead with the E3SM PR.

mark-petersen

This looks consistent with the CF-compliant changes that we've discussed. Thanks for your work on this.

xylar · 2022-07-26T18:24:46Z

@darincomeau, can we close this now that we have E3SM-Project#5027? I get 2 emails every time you push something to this branch.

darincomeau · 2022-07-26T18:27:35Z

Closing per E3SM-Project#5027

Make MPAS-Ocean units CF compliant This makes changes to the units in MPAS-Ocean's Registry files to be CF compliant. The main changes for CF compliance are: * variables not have a units attribute if they are unitless (i.e. no units='unitless', units='NA', etc.) * variables should have units=1 if they are dimensionless (e.g. real-valued coefficients) * we cannot use latex symbols like { and } in the units * we cannot use units='PSU' for salinity, we need to use units=1e-3, though we can state that the units are grams per kilogram in the description (used as the long_name attribute in output files). Further discussion found in E3SM-Ocean-Discussion#24 Note this does not change any units on BGC variables [BFB]

updating Icepack to hash e8e7d7d

tangq and others added 3 commits April 14, 2022 19:38

Increase the max number of EAM history tapes to 15

6fdd068

Modify the namelist variables to allow up to 15 EAM output tapes. [BFB] - Bit-For-Bit [NML] - Namelist Changing

Change code to support up to 15 EAM tapes

8f2b1c1

Cleanup of sea ice registry

5e902c9

Add CF compliant units to registry Change non-CF compliant units in registry to CF compliant ones Reformat Registry for clarity Moved incremental remapping fields to separate registry file

xylar reviewed May 26, 2022

View reviewed changes

xylar and others added 2 commits June 3, 2022 18:52

vanroekel approved these changes Jun 16, 2022

View reviewed changes

darincomeau added 4 commits June 16, 2022 14:41

Changing units="Sv" to units="Sverdrup" for CF compliance

82063a4

Removing units="" from Registry files, replacing with 1 where appropr…

37ba657

…iate

Removing units="none", replacing with 1 where appropriate

a3af233

Removing remaining instances of units="unitless" in analysis_members …

7f6949d

…Registry files

darincomeau force-pushed the darincomeau/ocn/cf-compliant-units branch from b38ebc1 to 7f6949d Compare June 16, 2022 19:43

darincomeau mentioned this pull request Jun 16, 2022

Make MPAS-Ocean units CF compliant E3SM-Project/E3SM#5027

Merged

darincomeau added 12 commits June 21, 2022 14:45

Replacing 1.e-3^-1 with 1.e3 in salinity units

d7ecf7c

Adding in units=1 or m as appropriate

3781464

Changing 'per second' in units to 's^-1'

315f611

More changes per second > s^-1

6f6b1ac

Changing degrees Celsius/degC to C in units

6f5aac3

Changing m m^-1 units to 1

ccb785a

Adding in more units=1 where appropriate

86413f5

Adding more units=1 or m as appropriate

a5c96a7

Replacing a missing end bracket accidentally removed in earlier commit

f19c3ae

Adding units=1 to some globalStats entries

f11e8ae

Phrasing of salinity description

a465fe2

Fixing space issue

a100be0

mark-petersen self-requested a review July 19, 2022 14:09

mark-petersen approved these changes Jul 19, 2022

View reviewed changes

darincomeau added 2 commits July 26, 2022 13:21

Changing remaining instances of units \circ C to C

0794027

Changing remaining units 'grams salt per kilograms seawater' to 1.e-3

7b18ec7

darincomeau closed this Jul 26, 2022

xylar pushed a commit that referenced this pull request Nov 8, 2023

Merge pull request #24 from eclare108213/update_icepack_5

f8716bd

updating Icepack to hash e8e7d7d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify MPAS-Ocean Registry files to have CF compliant units #24

Modify MPAS-Ocean Registry files to have CF compliant units #24

darincomeau commented May 24, 2022

darincomeau commented May 24, 2022

rljacob commented May 24, 2022

xylar commented May 25, 2022

xylar commented May 25, 2022

darincomeau commented May 25, 2022

xylar commented May 25, 2022

darincomeau commented May 25, 2022

darincomeau commented May 25, 2022

darincomeau commented May 25, 2022

czender commented May 25, 2022

xylar commented May 25, 2022

darincomeau commented May 26, 2022

xylar May 26, 2022

xylar May 26, 2022

xylar commented Jun 16, 2022

xylar commented Jun 16, 2022 •

edited

Loading

vanroekel left a comment

vanroekel commented Jun 16, 2022

xylar commented Jun 16, 2022

darincomeau commented Jun 16, 2022

xylar commented Jun 16, 2022

czender commented Jun 16, 2022

darincomeau commented Jun 16, 2022

darincomeau commented Jun 16, 2022

darincomeau commented Jun 16, 2022 •

edited

Loading

xylar commented Jun 16, 2022

mark-petersen left a comment

xylar commented Jul 26, 2022

darincomeau commented Jul 26, 2022

Modify MPAS-Ocean Registry files to have CF compliant units #24

Modify MPAS-Ocean Registry files to have CF compliant units #24

Conversation

darincomeau commented May 24, 2022

darincomeau commented May 24, 2022

rljacob commented May 24, 2022

xylar commented May 25, 2022

xylar commented May 25, 2022

darincomeau commented May 25, 2022

xylar commented May 25, 2022

darincomeau commented May 25, 2022

darincomeau commented May 25, 2022

darincomeau commented May 25, 2022

czender commented May 25, 2022

xylar commented May 25, 2022

darincomeau commented May 26, 2022

xylar May 26, 2022

Choose a reason for hiding this comment

xylar May 26, 2022

Choose a reason for hiding this comment

xylar commented Jun 16, 2022

xylar commented Jun 16, 2022 • edited Loading

vanroekel left a comment

Choose a reason for hiding this comment

vanroekel commented Jun 16, 2022

xylar commented Jun 16, 2022

darincomeau commented Jun 16, 2022

xylar commented Jun 16, 2022

czender commented Jun 16, 2022

darincomeau commented Jun 16, 2022

darincomeau commented Jun 16, 2022

darincomeau commented Jun 16, 2022 • edited Loading

xylar commented Jun 16, 2022

mark-petersen left a comment

Choose a reason for hiding this comment

xylar commented Jul 26, 2022

darincomeau commented Jul 26, 2022

xylar commented Jun 16, 2022 •

edited

Loading

darincomeau commented Jun 16, 2022 •

edited

Loading