Second rules update in v1 branch, update names, update write_standard_name_table.py to allow for subsections
#87
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"
mkavulich
requested review from
MayeulDestouches,
cacraigucar,
climbfuji,
dustinswales,
gold2718,
grantfirl,
mattldawson,
mwaxmonsky,
nusbaume,
peverwhee,
ss421 and
svahl991
as code owners
standard_names.xml
Outdated
| long_name="Molecular oxygen, O₂"> | ||
| </standard_name> | ||
| <standard_name name="ozone" | ||
| long_name="Ozone, O₃"> |
There was a problem hiding this comment.
Will these non-ASCII characters work? (also line 246)
There was a problem hiding this comment.
long_name should be able to handle any unicode characters since it is not used in any FORTRAN code context. Even if long names were eventually used in comments or even character variables, that would be fine according to the FORTRAN 90 standard*, so long as it's a character set supported by the OS. I think anything in UTF-32 should be allowed for long names, since Python can handle those characters and it's essentially universally supported among OS's.
This does bring up the fact that we haven't defined a character set for the standard names; I'll add that to the to-do list. So long as we make anything that may possibly be used as a fortran variable/subroutine/other code strictly within the allowed Fortran character set, we should be good.
- The relevant section on page 18 is here:
Additional characters may be representable in the processor, but may appear only in comments (3.3.1.1,
15 3.3.2.1), character constants (4.4.4), input/output records (9.1.1), and character string edit descriptors
16 (10.2.1).
There was a problem hiding this comment.
Can we talk about this at the next standard names meeting, please? My immediate reaction is to stick with the basic ASCII characters and nothing else, but maybe I can be convinced otherwise ;-) At the very least, we need to run a test with CCPP (SCM, UFS, ...) if the parsers (prebuild and capgen) can handle those characters.
There was a problem hiding this comment.
From today's discussion, we will restrict standard names (including long names) to ASCII character set.
| <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.
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?
There was a problem hiding this comment.
I think this is a good idea, but I will add it to my next round of changes to avoid scope creep. There are probably more abbreviations to introduce beyond this, like MYNN.
| <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.
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.
This is a good point: In general I think the use of "coupling" and "coupled" in variable names is ambiguous and problematic. Probably needs a deeper dive after this PR.
There was a problem hiding this comment.
We will use "for coupling" suffix, and include a detailed description of what that means (pending further potential discussion).
Also potentially need new suffixes depending on CESM use case, dependent on group names? Maybe drop "coupling" all together in favor of "timing"/"order" suffixes
gold2718
left a comment
gold2718
left a comment
There was a problem hiding this comment.
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.
mkavulich
left a comment
mkavulich
left a comment
There was a problem hiding this comment.
@gold2718 @climbfuji Thanks for your thorough reviews. I've addressed most of your comments/suggestions, and those that I haven't I replied with a follow-up comment or question. Let me know if anything else needs resolving.
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Great work @mkavulich! I had a couple of change requests, but nothing that requires a full re-review.
| comment="These names are used as bases for other names, but may\n | ||
| 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" |
There was a problem hiding this comment.
Since these can be considered formal standard names should we include the units here as well? (I'm happy to hold this off for a future PR if that would be easier).
|
Will #86 be merged in before final review? |
|
@gold2718 I did not plan to resolve conflicts between the main branch and release branch at this time, since the changes are already so divergent. But this is a good idea; I will make a follow-up PR right now to bring in the relevant changes since this branch was cut. I thought I had responded to your latest comments but I guess I forgot; I made the changes you requested re: vis to visible, was there anything else that needed attention or are you okay with this being merged? |
|
This PR changes many of the names that we just adapted in JEDI last fall during a code sprint that took considerable effort. Before merging this, can we create a tag or version of this repository that JEDI can reference? |
Please just open issues for any changes that need changing (based on comments on this PR) so that they are not lost. I do not see one for the |
No objections here. |
@svahl991 Just a note, this PR is not being merged to We had discussed creating a tag back in the fall for JEDI's "stable" version but last time we had that discussion there were still some changes being debated. Is the current state of main the preferred version to make this tag? Or would it be a previous hash? |
|
@gold2718 I have opened some issues that I think covers all of your concerns; I have been tracking them in a google doc but making sure they are tracked here as well is a good idea. |
Thanks @mkavulich . That's an important detail I missed. So, to make sure I understand:
I apologize if I missed and/or forgot past communications about this. |
gold2718
left a comment
gold2718
left a comment
There was a problem hiding this comment.
All looks good now, thanks!
(and thanks to @mkavulich for getting all those issues open)
| * `volumetric_soil_moisture_between_soil_bottom_and_water_table`: Volumetric soil moisture between soil bottom and water table | ||
| * `real(kind=kind_phys)`: units = m3 m-3 | ||
| * `water_vapor_mixing_ratio_wrt_moist_air_at_2m`: mixing ratio of the mass of water vapor to the mass of moist air, at two meters above surface | ||
| * `water_vapor_mixing_ratio_wrt_moist_air_at_2m`: Specific humidity (water vapor mass mixing ratio with respect to moist air) at two meters above surface |
There was a problem hiding this comment.
We have found in our work on JEDI that the term "specific humidity" does not have an internationally agreed-upon meaning. I suggest not using it at all to avoid confusion. Or, if it must be used, to qualify it. (When the UKMO uses the term "specific humidity" they mean water_vapor_mixing_ratio_wrt_moist_air_and_condensed_water, which is two lines below this one.)
|
@svahl991 Your understanding is correct. This re-factor is part of a lead-up to standing up a more formal governance outside of the CCPP framework's purview; we don't yet have anything official but I have been putting out feelers to ensure that all interested groups are represented. As an active contributor from JCSDA, I'll certainly be sure you're in the loop when we start scheduling meetings. With regards to specific humidity, you'll note that this PR removes |
## Description This PR merges The `release/v1` branch into the `main` branch. The `release/v1` branch was split off from `main` about a year ago, with the intention to make major rules and name changes to improve the consistency and maintainability of both the rules and the names, without giving major inconveniences/disruption to those currently making use of the main branch. After a year of discussion and changes, it is time to bring these changes to their final resting place in the main branch. The major breaking and/or non-back-compatible changes can be summarized as such (See the subsections below for specific details about these changes): - the physics `kind` field is removed - the `long_name` field is changed to `description` - Several changes to particular terms and components of existing names have been made After this PR is merged, a `v1.0.0` tag will be created, representing the first true "versioned" version of the ESM Standard Names. While more rule changes are likely in the future to resolve open and future issues, this should be a more stable jumping-off point to allow updates and reconciliation with the names in the CCPP physics repository, which has not been resolved in many years now. For those who have not been following along with the discussion and changes related to the **v1** branch, here is a summary of each of the changes made on this branch: ### #85 First rules update, fixing misspelled standard names This first change introduced some changes to the Rules document based on discussion in the CCPP framework regular meetings. These rules changes can be summarized as follows - Introduced a more rigorous and standardized formula for constructing new standard names, with specific rules and definitions of each component of the name, attempting to cover all possible cases - Introduced the concept of "suffixes" to compliment prefixes, with mixing_ratio_wrt``_Y`` being the first example - Introduced the concept of "Reserved phrases"...for now only including "CCPP" as a reference to CCPP-specific variables In addition, a large number of misspellings within the existing names and rules were fixed. ### #87 Second rules update in v1 branch, update several name types This second change introduced the concept of "base names"; representing the main entity from which a standard name is constructed. Some existing prefixes (`surface_X` and `air_X`) were converted to suffixes (`X_at_surface` and `X_of_air`) to improve consistency with other existing names and rules, and some superfluous `surface` wording was removed from several names. The definitions for mixing ratios were improved, and the rules for constructing new names were updated and improved. ### #104 Add techincal specification, substitute abbreviations, include base name definitions This third rules change included some info about technical specifications of the standard names repository, and some formatting improvements. Instances of the term `weight` were changed to `scaling_factor` to avoid potential confusion with the physical property of weight. `long_name` descriptions were added for all the new "base names" with a few minor exceptions. Some new abbreviations were defined to help shorten names. Some unused and duplicate entries were removed. Finally, CCPP-specific variables were consolidated into their own section. ### #116 Rename long_name --> description, update description rules, expand list of abbreviations This fourth change renamed the `long_name` field to `description`, clarifying that this field should be unique, and improving/fixing some existing descriptions. Some more duplicate entries were removed. Some additional new abbreviations were defined to continue shortening names. Finally, continued defining more abbreviations to help shorten standard names further. ### #124 Remove `kind` entry, clarify rules for units and disallowed terms This fifth and final change to the v1 branch (aside from another PR to resolve intervening changes from the main branch) updated the rules to clarify disallowed terms and the role of `units`, and removed the `kind` entry. The `constants` section was organized alphabetically, and changed some names regarding `dry_air`. ## Issues Resolves - #48 - #68 - #94 - #102 Also reference already-closed issues: - #92 - #93 - #95 - #103 --------- Co-authored-by: Jesse Nusbaumer <[email protected]> Co-authored-by: Dom Heinzeller <[email protected]>
5 participants
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.pyYou 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.