Merge feature/capgen into main as of 2024-03-08 #546
Conversation
…ited to unit conversions
…to metavar.py and var_props.py
…n_capgen feature/capgen: add new tests for unit conversions
Fix unit converter problem with units = 1
Merge main into feature/capgen (merge after NCAR#493)
… fix spelling: var_compatability --> var_compatibility (NCAR#512) * Add capability to parse Fortran pointer variables to fortran_tools/parse_fortran.py * Add --debug option to capgen (framework_env.py) * For advection tests, add --debug flag to capgen and update test reports * Add active attribute to variable water_vapor_specific_humidity in capgen_test, add --debug flag to capgen and update test reports * For var_action tests, add --debug flag to capgen and update test reports * Add docstring documentation for add_var_debug_check and write_var_debug_check in scripts/suite_objects.py * Bug fix in scripts/suite_objects.py: also check variable allocations for variables local to the group and assign correct dimensions for local (group) and module (suite) variables * Fix spelling: compatability --> compatibility --------- Co-authored-by: mwaxmonsky <137746677+mwaxmonsky@users.noreply.github.com>
Expand var compatibility test for active attribute
NCAR#531) Enable 1D variables with only a vertical dimension by removing code that always adds a dimension transform. var_props.py: adjust logic that always adds vertical transform and is prohibiting 1D variables with a vertical coordinate --------- Co-authored-by: Courtney Peverley <courtneyp@derecho5.hsn.de.hpc.ucar.edu>
Updates to enable more than one scheme group, and also enable unit transforms for scalar (non-dimensioned) variables. Updated Files Groups: ccpp_suite.py - don't promote loop variables to suite level host_cap.py - fix for iterating over suite part list suite_objects.py - check if variable is at suite level before giving up hope and throwing an error Scalar transforms: suite_objects.py - don't try to do vertical dimension transform for scalar variable; conditionally write comments for var transform calls var_props.py - don't include parenthesis (e.g. variable1() = 2 * variable2()) for scalar variable transforms Misc: metavar.py - fix for bug surely introduced by me converting to fstrings at some point
This PR adds support to use the fortran optional attribute at the scheme level in Capgen. This functionality does not replace the active attribute, but rather builds on it. Previously, conditionally allocated fields needed by a scheme were required to be accompanied with the allocation logic defined by the host (e.g the "active" condition). With this change, the scheme can internally query the "presence" instead of carrying the host model logic "down into the scheme". The following modifications were made: Metadata parsing: Check for compatibility between scheme and host metadata. If host has inactive variable, ensure that scheme has variable declared as optional. Fortran parsing: Check for consistency between metadata and fortran file. If fortran file does not contain the optional attribute in its declaration, report error. For any optional scheme variable, add optional attribute to cap level declarations. Declare null pointer at cap level when host does not use optional scheme variable
… into feature/merge_feature_capgen_into_main_20240308
… in unit metadata attribues
climbfuji
changed the title
…ature/merge_feature_capgen_into_main_20240308
climbfuji
requested review from
grantfirl,
gold2718 and
mkavulich
as code owners
There was a problem hiding this comment.
All tests pass and allof this code has been reviewed except for 6cdd38a, which looks fine to me.
| _NEGATIVE_NON_LEADING_ZERO_NUM = f"[-]{_NON_LEADING_ZERO_NUM}" | ||
| _UNIT_EXPONENT = f"({_NEGATIVE_NON_LEADING_ZERO_NUM}|{_NON_LEADING_ZERO_NUM})" | ||
| _UNIT_REGEX = f"[a-zA-Z]+{_UNIT_EXPONENT}?" | ||
| _UNITS_REGEX = f"^({_CHAR_WITH_UNDERSCORE}|{_UNIT_REGEX}(\s{_UNIT_REGEX})*|{_UNITLESS_REGEX})$" |
There was a problem hiding this comment.
The only commit on top of the merge is 6cdd38a, which is required to parse the metadata in the Unified Forecast System / ccpp-physics (underscores in units, e.g. degree_north).
Is there a plan to resolve this discrepancy?
There was a problem hiding this comment.
Do you mean bringing this bug fix back to the feature/capgen branch? Oh yes, I am going to do that in the next days by cherry-picking the commit and creating a PR for feature/capgen.
There was a problem hiding this comment.
Note that this is not a bug in the UFS, units with underscores are in the CCPP standard names dictionary: search for degree_ in https://github.com/ESCOMP/CCPPStandardNames/blob/main/Metadata-standard-names.md
There was a problem hiding this comment.
Is degree_north in UDUNITS? If so, nevermind.
There are lots of ways to specify location, I don't think we need to introduce variances in the CCPP.
There was a problem hiding this comment.
The CF conventions use degrees_north (note the plural) and since CCPP standard names are extensions of those we need to support the underscore, too. But do not ask me why CCPP standard names, UFS etc use the singular version and not the plural version - maybe @grantfirl knows (either way it's out of scope of this PR, the underscore is the only thing that matters here).
There was a problem hiding this comment.
FWIW. CF allows for several location descriptions:
|
Hello, @grantfirl @dustinswales , testing is complete on UFS-WM PR#2181. Can you please merge this sub-PR ? |
Update main from feature/capgen as of 2024-03-08 (includes optional argument updates in
feature/capgen).The only commit on top of the merge is 6cdd38a, which is required to parse the metadata in the Unified Forecast System / ccpp-physics (underscores in units, e.g.
degree_north).User interface changes?: No
Fixes: Working towards #540 (need the changes to the metadata parser to implement optional arguments in
ccpp_prebuild)Testing:
capgen
- in
./test:./pylint_test.shworks and gives "similar" results as before (see #547)- in
./test:./run_fortran_tests.shall pass- in
scripts:python3 -m doctest *.pyno errorsprebuild
- in
./test_prebuild:test_mkstatic.pyworks- in
./test_prebuild:test_metadata_parser.pyworks- in
./test_prebuild/test_blocked_data: tests pass- in
./test_prebuild/test_chunked_data: tests passufs-weather-model
- See ufs-community/ufs-weather-model#2181
Associated PRs:
ufs-community/ccpp-physics#182
NOAA-EMC/fv3atm#796
ufs-community/ufs-weather-model#2181