Make MPAS-Ocean units CF compliant #5027

darincomeau · 2022-06-16T21:12:28Z

This makes changes to the units in MPAS-Ocean's Registry files to be CF compliant. CF Standard Units

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, as found in the following Registry files under tracer_groups:

Registry_CFC
Registry_DMS
Registry_MacroMolecules
Registry_ecosys
Registry_idealAge

[BFB]

xylar

Wow, @darincomeau! This was a lot of work! Thanks for putting in the time. I went through all the files and found ~80 things to fix, which sounds like a lot but it's a tiny fraction compared to what you changed already.

components/mpas-ocean/src/Registry.xml

components/mpas-ocean/src/analysis_members/Registry_global_stats.xml

components/mpas-ocean/src/analysis_members/Registry_mixed_layer_heat_budget.xml

xylar · 2022-06-21T09:55:07Z

components/mpas-ocean/src/analysis_members/Registry_zonal_mean.xml

@@ -50,10 +50,10 @@
 		<var name="binBoundaryZonalMean" type="real" dimensions="nZonalMeanBinsP1" units="varies"


@czender, this variable sometimes gets used with units of radians and sometimes with m. Is there a way to handle this in a CF-compliant way?

Just noting this is the only review comment I haven't addressed

components/mpas-ocean/src/tracer_groups/Registry_activeTracers.xml

components/mpas-ocean/src/Registry.xml

components/mpas-ocean/src/tracer_groups/Registry_activeTracers.xml

czender

Looks great. I agree with @xylar on replacing the "per second" with "s^-1" or "second-1", and that inverse salinity is better as "1.e^3" than "1.e^-3^-1".

darincomeau · 2022-06-23T00:05:21Z

Thanks again @xylar for all of those comments! I've corrected them all, along with some other similar offenders that came up with grep.
I also reran this branch through your new compass_daily_monthly to make sure nothing broke and it passes.

mark-petersen

Thanks @darincomeau. This looks great. I ran a quick test to make sure. This compiles with both gnu and intel, and passes gnu optimized bfb against the branchpoint (as expected) with the compass nightly suite.

rljacob · 2022-06-30T17:19:32Z

@xylar is this ready?

xylar · 2022-06-30T17:23:06Z

@rljacob, no these changes are big enough that I will take another look when I get back from vacation and I do want all the reviewers listed here to have a look, too. @milenaveneziani and @vanroekel, can you have a look when you have time?

xylar

Thanks, @darincomeau, the updated version looks great!

xylar · 2022-07-12T14:40:42Z

components/mpas-ocean/src/analysis_members/Registry_global_stats.xml

@@ -52,7 +52,7 @@
 			 description="sum of the volumeEdge variable over the full domain, used to normalize global statistics"
 			 packages="forwardMode;analysisMode"
 		/>
-		<var name="CFLNumberGlobal" type="real" dimensions="Time" units="unitless"
+		<var name="CFLNumberGlobal" type="real" dimensions="Time"units="1"


Space issue

Suggested change

<var name="CFLNumberGlobal" type="real" dimensions="Time"units="1"

<var name="CFLNumberGlobal" type="real" dimensions="Time" units="1"

nice catch - thanks!

darincomeau · 2022-07-26T14:51:58Z

@milenaveneziani @vanroekel , this is good to go as far as I and the approved reviewers. Would you mind taking a look here, since there are extensive changes?

milenaveneziani · 2022-07-26T17:00:35Z

components/mpas-ocean/src/Registry.xml

 					description="Form of pressure gradient terms in momentum equation. For most applications, the gradient of pressure and layer mid-depth are appropriate.  For isopycnal coordinates, one may use the gradient of the Montgomery potential.  The sea surface height gradient (ssh_gradient) option is for barotropic, depth-averaged pressure."
 					possible_values="'ssh_gradient', 'pressure_and_zmid' or 'Jacobian_from_density' or 'Jacobian_from_TS' or 'MontgomeryPotential' or 'constant_forced'"
 		/>
-		<nml_option name="config_common_level_weight" type="real" default_value="0.5" units="unitless"
+		<nml_option name="config_common_level_weight" type="real" default_value="0.5"
 					description="The weight between standard Jacobian and weighted Jacobian, $\gamma$."


Is the use of the latex '$$' allowed? (I noticed this here, but there might be other places that I might have missed)

Good call! Those need to be removed because they just clutter things up now that we're not focusing on the latex documentation. My guess is that they won't cause errors in the CF-checker though.

@darincomeau, I suppose it's up to you if that's in the scope of this PR or should be a separate one.

Thanks for pointing this out @milenaveneziani . But I think trying to remove all latex syntax from attributes like description, which won't impact CF compliance, are more general registry cleanup and outside the scope here, so I think I'd prefer to leave this as is for now rather than trying to do a more complete job of that.

sounds good @darincomeau. I just wanted to make sure that it wasn't required.

components/mpas-ocean/src/Registry.xml

milenaveneziani · 2022-07-26T17:10:10Z

components/mpas-ocean/src/analysis_members/Registry_high_frequency_output.xml

 			description="divergence of transport velocity at cell centers at a depth of approximately 250 m"
 		/>
-		<var name="relativeVorticityVertexAt250m" type="real" dimensions="nVertices Time" units="s^{-1}"
+		<var name="relativeVorticityVertexAt250m" type="real" dimensions="nVertices Time" units="s^-1"
 			description="relative vorticity at cell vertices at a depth of approximately 250 m"
 		/>
 		<var_array name="activeTracersAtSurface" type="real" dimensions="nCells Time">
 				<var name="temperatureAtSurface" array_group="activeTracersAtSurface" units="^\circ C" name_in_code="temperatureAtSurface"


Should be 'C' above.

Definitely a great catch here.

milenaveneziani · 2022-07-26T17:10:35Z

components/mpas-ocean/src/analysis_members/Registry_high_frequency_output.xml

 					description="relative vorticity at cell centers at bottom"
 			/>
-			<var name="divergenceAtBottom" type="real" dimensions="nCells Time" units="s^{-1}"
+			<var name="divergenceAtBottom" type="real" dimensions="nCells Time" units="s^-1"
 					description="divergence at cell centers at bottom"
 		/>
 		<var_array name="activeTracersAvgTopto0100" type="real" dimensions="nCells Time">
 				<var name="temperatureAvgTopto0100" array_group="activeTracersAvgTopto0100" units="^\circ C" name_in_code="temperatureAvgTopto0100"


same here. I also noticed a couple of more places below, so you may want to check the whole file.

and here... I'll grep for the offender to fix any others

milenaveneziani · 2022-07-26T17:12:59Z

components/mpas-ocean/src/analysis_members/Registry_lagrangian_particle_tracking.xml

 			 description="flag to monitor if the particle was transfered"
 		/>
 <!-- TEMP FIELDS THAT NEED MOVED TO NonHalo below after verifying interpolation is correct -->
-		<var name="particleTemperature" type="real" dimensions="nParticles Time" units="degrees Celsius"
+		<var name="particleTemperature" type="real" dimensions="nParticles Time" units="C"
 			 description="sampled temperature for particle"
 		/>
 		<var name="particleSalinity" type="real" dimensions="nParticles Time" units="grams salt per kilogram seawater"


change units above

thanks! will do

milenaveneziani

wow @darincomeau: these are indeed extensive changes.. Thanks for implementing them.
I noted a few things by going through each file: not sure if I got everything, but hopefully..

darincomeau · 2022-07-26T18:26:10Z

Thanks for those catches @milenaveneziani - changes have been pushed. I think I've said this earlier in this PR, but I've only actually run cfchecker on the output from the daily output test (which now matches monthly output), so while I do grep for violations and change all I find, there are bound to be some missed.

rljacob · 2022-08-04T17:18:36Z

@vanroekel please review.

vanroekel

looks good to me. Very impressive work @darincomeau

jonbob · 2022-08-08T19:16:15Z

@darincomeau - can you please check the conflicts in Registry? I'd like to merge this and I'm not sure about some of the changes that have been introduced since you started this work

…iate

…Registry files

darincomeau · 2022-08-08T19:50:07Z

Rebased to pick up and resolve conflicts in Registry.xml.

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]

jonbob · 2022-08-08T22:13:58Z

passed:

ERS.ne11_oQU240.WCYCL1850NS.chrysalis_intel
SMS_D_Ld3.T62_oQU120.CMPASO-IAF.chrysalis_intel
PEM_Ln9.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel

merged to next

jonbob · 2022-08-09T19:15:14Z

merged to master

darincomeau added mpas-ocean BFB PR leaves answers BFB labels Jun 16, 2022

darincomeau requested review from mark-petersen, xylar, milenaveneziani and vanroekel June 16, 2022 21:12

darincomeau assigned jonbob Jun 16, 2022

xylar requested changes Jun 21, 2022

View reviewed changes

xylar reviewed Jun 21, 2022

View reviewed changes

components/mpas-ocean/src/Registry.xml Outdated Show resolved Hide resolved

xylar reviewed Jun 21, 2022

View reviewed changes

components/mpas-ocean/src/tracer_groups/Registry_activeTracers.xml Outdated Show resolved Hide resolved

darincomeau requested a review from czender June 21, 2022 15:15

czender approved these changes Jun 21, 2022

View reviewed changes

mark-petersen approved these changes Jun 26, 2022

View reviewed changes

xylar approved these changes Jul 12, 2022

View reviewed changes

milenaveneziani reviewed Jul 26, 2022

View reviewed changes

components/mpas-ocean/src/Registry.xml Show resolved Hide resolved

milenaveneziani reviewed Jul 26, 2022

View reviewed changes

milenaveneziani approved these changes Jul 26, 2022

View reviewed changes

xylar mentioned this pull request Jul 26, 2022

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

Closed

vanroekel approved these changes Aug 4, 2022

View reviewed changes

Removing curly braces from negative exponents in units, in Registry.xml

01c95a4

darincomeau added 20 commits August 8, 2022 14:41

Removing single instance of units=NA in analysis_members Registry files

c9c2ed9

Removing instances of units=NA/nondimensional, or replacing with units=1

de872db

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

d55daf2

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

fa76284

…iate

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

6625775

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

cca5806

…Registry files

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

dba18ee

Adding in units=1 or m as appropriate

d8d7e49

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

7144220

More changes per second > s^-1

31e5715

Changing degrees Celsius/degC to C in units

81b7c4a

Changing m m^-1 units to 1

cca3834

Adding in more units=1 where appropriate

67936ce

Adding more units=1 or m as appropriate

18b1547

Replacing a missing end bracket accidentally removed in earlier commit

2b1e1aa

Adding units=1 to some globalStats entries

0e88143

Phrasing of salinity description

e94b0f4

Fixing space issue

9eeca6a

Changing remaining instances of units \circ C to C

a428f39

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

c577c12

darincomeau force-pushed the darincomeau/ocn/cf-compliant-units branch from 7b18ec7 to c577c12 Compare August 8, 2022 19:49

jonbob merged commit cd9f5d1 into master Aug 9, 2022

jonbob deleted the darincomeau/ocn/cf-compliant-units branch August 9, 2022 19:15

mark-petersen mentioned this pull request Sep 6, 2022

add carbon to conservation check analysis member #5151

Merged

darincomeau mentioned this pull request Aug 30, 2022

Add WAVEWATCH III as new coupled wave component #5047

Merged

xylar mentioned this pull request Nov 19, 2022

Update E3SM-Project submodule MPAS-Dev/compass#461

Merged

32 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make MPAS-Ocean units CF compliant #5027

Make MPAS-Ocean units CF compliant #5027

darincomeau commented Jun 16, 2022 •

edited

Loading

xylar left a comment

xylar Jun 21, 2022

darincomeau Jun 23, 2022

czender left a comment

darincomeau commented Jun 23, 2022

mark-petersen left a comment

rljacob commented Jun 30, 2022

xylar commented Jun 30, 2022

xylar left a comment

xylar Jul 12, 2022

darincomeau Jul 12, 2022

darincomeau commented Jul 26, 2022

milenaveneziani Jul 26, 2022

xylar Jul 26, 2022

darincomeau Jul 26, 2022

milenaveneziani Jul 26, 2022

milenaveneziani Jul 26, 2022

darincomeau Jul 26, 2022

milenaveneziani Jul 26, 2022 •

edited

Loading

darincomeau Jul 26, 2022

milenaveneziani Jul 26, 2022

darincomeau Jul 26, 2022

milenaveneziani left a comment

darincomeau commented Jul 26, 2022

rljacob commented Aug 4, 2022

vanroekel left a comment

jonbob commented Aug 8, 2022

darincomeau commented Aug 8, 2022

jonbob commented Aug 8, 2022

jonbob commented Aug 9, 2022

		@@ -50,10 +50,10 @@
		<var name="binBoundaryZonalMean" type="real" dimensions="nZonalMeanBinsP1" units="varies"

	<var name="CFLNumberGlobal" type="real" dimensions="Time"units="1"
	<var name="CFLNumberGlobal" type="real" dimensions="Time" units="1"

Make MPAS-Ocean units CF compliant #5027

Make MPAS-Ocean units CF compliant #5027

Conversation

darincomeau commented Jun 16, 2022 • edited Loading

xylar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

czender left a comment

Choose a reason for hiding this comment

darincomeau commented Jun 23, 2022

mark-petersen left a comment

Choose a reason for hiding this comment

rljacob commented Jun 30, 2022

xylar commented Jun 30, 2022

xylar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darincomeau commented Jul 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milenaveneziani Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milenaveneziani left a comment

Choose a reason for hiding this comment

darincomeau commented Jul 26, 2022

rljacob commented Aug 4, 2022

vanroekel left a comment

Choose a reason for hiding this comment

jonbob commented Aug 8, 2022

darincomeau commented Aug 8, 2022

jonbob commented Aug 8, 2022

jonbob commented Aug 9, 2022

darincomeau commented Jun 16, 2022 •

edited

Loading

milenaveneziani Jul 26, 2022 •

edited

Loading