-
Notifications
You must be signed in to change notification settings - Fork 16
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
Second rules update in v1 branch, update names, update write_standard_name_table.py
to allow for subsections
#87
base: release/v1
Are you sure you want to change the base?
Conversation
…xner_function --> exner_function, add long names where appropriate
…rd names (keep in long names for reference), slightly revise rules on the proper term to align with other uses of "mixing ratio"
|
||
This construction was originally based on rules set forth in the | ||
`CF guidelines <http://cfconventions.org/Data/cf-standard-names/docs/guidelines.html>`_), | ||
but have since evolved for better consistency and generality across a broader set of fields | ||
than was originally envisioned by the CF conventions. "``medium``" should be specified when | ||
the variable in question is a substance or other quantity contained within some other medium | ||
(e.g. for "mole_fraction_of_ozone_in_air", the base name is "ozone", while the medium is "air"). | ||
the variable in question is a substance or other quantity contained within some other the medium |
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.
The added "the" doesn't make sense here
@@ -429,13 +446,13 @@ Special phrases | |||
+------------------------+-------------------------------------------------------------------------------------+ | |||
|frozen_water | ice | | |||
+------------------------+-------------------------------------------------------------------------------------+ | |||
| longwave | longwave radiation | | |||
| longwave | Longwave radiation. Defined as thermal emission of EM radiation from the planet. | |
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 either spell out electromagnetic or drop EM altogether
+------------------------+-------------------------------------------------------------------------------------+ | ||
| moisture | water in all phases contained in soil | | ||
+------------------------+-------------------------------------------------------------------------------------+ | ||
| ocean | used instead of in_sea_water for quantities which are large-scale rather than local | | ||
+------------------------+-------------------------------------------------------------------------------------+ | ||
| shortwave | shortwave radiation | | ||
| shortwave | Shortwave radiation. Defined as EM emissions from the sun | |
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.
ditto
long_name="coefficient"> | ||
<type units="1">real</type> | ||
</standard_name> | ||
<standard_name name="coefficient" |
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.
Duplicate of line 26-29
long_name="Molecular oxygen, O₂"> | ||
</standard_name> | ||
<standard_name name="ozone" | ||
long_name="Ozone, O₃"> |
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.
Will these non-ASCII characters work? (also line 246)
@@ -2311,7 +2831,8 @@ | |||
<standard_name name="upward_virtual_potential_temperature_flux"> | |||
<type kind="kind_phys" units="K m s-1">real</type> | |||
</standard_name> | |||
<standard_name name="surface_upward_specific_humidity_flux_for_mellor_yamada_janjic_surface_layer_scheme"> | |||
<standard_name name="upward_flux_of_water_vapor_mixing_ratio_wrt_moist_air_at_surface_for_mellor_yamada_janjic_surface_layer_scheme" |
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.
We should probably define MYJ as the abbreviation to use in the standard name, and the full name in the long name? Like PBL and GWD in my open PR?
<type kind="kind_phys" units="Pa">real</type> | ||
</standard_name> | ||
<standard_name name="control_for_surface_layer_evaporation"> | ||
<type kind="kind_phys" units="1">real</type> | ||
</standard_name> | ||
<standard_name name="surface_specific_humidity_for_myj_schemes"> | ||
<standard_name name="water_vapor_mixing_ratio_wrt_moist_air_at_surface_for_myj_schemes" |
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.
Here, MYJ should be uppercase in the standard name, and spelled out in the long name
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.
Rule 17 says names are insensitive, does it matter here?
If so, maybe we should add something and uppercase the abbreviations in the Acronyms, Abbreviations, and Aliases section? Or is it just for initials of names?
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.
That (rule 17) is a good point I completely forgot about. I like consistency and would prefer either lowercase or uppercase. We can talk about it at one of the next meetings, if the majority thinks that case insensitive is fine, then I can live with that, too.
<type kind="kind_phys" units="fraction">real</type> | ||
</standard_name> | ||
<standard_name name="lwe_surface_snow_from_coupled_process"> | ||
<type kind="kind_phys" units="m">real</type> | ||
</standard_name> | ||
<standard_name name="surface_upward_latent_heat_flux_from_coupled_process"> | ||
<standard_name name="upward_latent_heat_flux_at_surface_from_coupled_process"> |
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.
What is the difference between for_coupling
and from_coupled_process
(not for vs from, but coupling vs coupled process)?
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.
Great (and great big) piece of work!
A few requested changes and a lot of questions along with a few suggestions.
Also, should all long names begin with an capitalized word? Right now, there is a mix.
for std_name in sec: | ||
if std_name.tag == 'section': | ||
parse_section(snl, std_name, level + '#') | ||
continue |
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.
Why use continue
instead of a regular else
clause?
more specific standard names.\n"> | ||
<standard_name name="amount" | ||
long_name="amount"> | ||
<type units="kg m-2">real</type> |
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.
What is the role of specific units here? Can't the units be different in the full standard name, or more precisely, in a variable with the standard name? Do we need the units in this section?
<standard_name name="air_temperature/water_vapor" | ||
long_name="air_temperature/water_vapor"> | ||
</standard_name> |
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.
This is not a legal standard name (rule 16) and the long name is not helping me understand what it is.
also be considered standard names on their own. See the\n | ||
full list of standard names for further details.\n"> | ||
<standard_name name="absolute_vorticity" | ||
long_name="absolute_vorticity"> |
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.
The long names here and below are supposed to be descriptive and do not need the underscores. If you are just going to copy the standard name, please omit the long name.
<standard_name name="water" | ||
long_name="water"> | ||
</standard_name> |
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.
What is this? How is it related to all the other water variables?
@@ -2431,7 +2962,8 @@ | |||
<standard_name name="weight_for_potential_temperature_at_top_of_viscous_sublayer"> | |||
<type kind="kind_phys" units="1">real</type> | |||
</standard_name> | |||
<standard_name name="weight_for_specific_humidity_at_top_of_viscous_sublayer"> | |||
<standard_name name="weight_for_water_vapor_mixing_ratio_wrt_moist_air_at_top_of_viscous_sublayer" | |||
long_name="Weight for specific humidity (water vapor mass mixing ratio with respect to moist air) at the top of the viscous sublayer"> |
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.
Is this "weight" as in "weighting factor"? If so, could "weighting factor" be used in the long name? If not, why are the units "1"?
@@ -2589,16 +3125,20 @@ | |||
<standard_name name="max_vegetation_area_fraction"> | |||
<type kind="kind_phys" units="fraction">real</type> | |||
</standard_name> | |||
<standard_name name="nir_albedo_strong_cosz"> | |||
<standard_name name="nir_albedo_strong_cosz" |
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.
Does cosz
need to be in the Acronyms, Abbreviations, and Aliases section?
<standard_name name="direct_uv_and_vis_shortwave_flux" | ||
long_name="direct_uv_and_vis_shortwave_flux"> | ||
</standard_name> | ||
<standard_name name="direct_visible_albedo" |
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.
Is there a reason there are some uses of visible
instead of vis
?
<type kind="kind_phys" units="J m-2">real</type> | ||
</standard_name> | ||
<standard_name name="cumulative_surface_downwelling_diffuse_uv_and_vis_shortwave_flux_for_coupling_multiplied_by_timestep"> | ||
<standard_name name="cumulative_downwelling_diffuse_uv_and_vis_shortwave_flux_at_surface_for_coupling_multiplied_by_timestep"> |
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 think it would be helpful to spell out uv_and_vis_shortwave
in a long name:
<standard_name name="cumulative_downwelling_diffuse_uv_and_vis_shortwave_flux_at_surface_for_coupling_multiplied_by_timestep"> | |
<standard_name name="cumulative_downwelling_diffuse_uv_and_vis_shortwave_flux_at_surface_for_coupling_multiplied_by_timestep" | |
long_name="Cumulative downwelling diffuse ultraviolet_and_visible parts of the shortwave flux at surface for coupling multiplied by timestep"> |
This suggestion also applies to other uses of uv_and_vis_shortwave
.
<type kind="kind_phys" units="J m-2">real</type> | ||
</standard_name> | ||
<standard_name name="cumulative_surface_downwelling_direct_nir_shortwave_flux_for_coupling_multiplied_by_timestep"> | ||
<standard_name name="cumulative_downwelling_direct_nir_shortwave_flux_at_surface_for_coupling_multiplied_by_timestep"> |
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 think it would be helpful to spell out nir_shortwave in a long name:
<standard_name name="cumulative_downwelling_direct_nir_shortwave_flux_at_surface_for_coupling_multiplied_by_timestep"> | |
<standard_name name="cumulative_downwelling_direct_nir_shortwave_flux_at_surface_for_coupling_multiplied_by_timestep" | |
long_name="Cumulative downwelling direct near infrared portions shortwave flux at surface for coupling multiplied by timestep"> |
This suggestion also applies to other uses of `nir_shortwave,
This is the second round of proposed rules changes based on our ongoing discussion to make both rules and names as consistent as possible. Major updates include:
Standard Names
Rules
write_standard_name_table.py
You can see these changes in the form of a Google doc for better visualization here: https://docs.google.com/document/d/19ysUCWDhv53W8fQbElW7opr_1Pm7ck95QRUyKM_qy4E/edit?tab=t.0
Note: with this update, we have 347 standard names that are fully compliant with the rules we have set out. A big portion of the remainder are "flag"-type variables, indices, etc, which are not fully accounted for in the rules yet.