From 455003c010c2c720f58574dc4dfdde2e7d05b0c6 Mon Sep 17 00:00:00 2001 From: Courtney Peverley Date: Wed, 6 Nov 2024 16:37:34 -0700 Subject: [PATCH] Add new register phase (#319) Tag name (required for release branches): Originator(s): peverwhee Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number): 1. Adds call to new CCPP register phase brought in with framework PR [#582](https://github.com/NCAR/ccpp-framework/pull/582) 2. Uses the full error message returned from the exception handled when xmllint is called (updated in framework PR [#586](https://github.com/NCAR/ccpp-framework/pull/586)) 3. Adds `inverse_exner_function_wrt_surface_pressure` as possible input variable name for exner for backwards compatibility with old converted snapshots closes #215 (add optional register phase) closes #286 (Improve error message returned by XML linter) Describe any changes made to build system: None Describe any changes made to the namelist: None List any changes to the defaults for the input datasets (e.g. boundary datasets): None List all files eliminated and why: None List all files added and what they do: None List all existing files that have been modified, and describe the changes: (Helpful git command: `git diff --name-status development...`) M .gitmodules - brings in new CCPP framework tag M src/control/cam_comp.F90 M src/physics/utils/phys_comp.F90 - Adds call to new CCPP register phase in the generated cap M src/data/generate_registry_data.py - Uses full error message returned from xmllint M src/data/registry.xml - add backwards-compatible exner name If there are new failures (compared to the `test/existing-test-failures.txt` file), have them OK'd by the gatekeeper, note them here, and add them to the file. If there are baseline differences, include the test and the reason for the diff. What is the nature of the change? Roundoff? derecho/intel/aux_sima: all pass derecho/gnu/aux_sima: all pass If this changes climate describe any run(s) done to evaluate the new climate in enough detail that it(they) could be reproduced: N/A CAM-SIMA date used for the baseline comparison tests if different than latest: --------- Co-authored-by: Courtney Peverley --- .gitmodules | 2 +- ccpp_framework | 2 +- src/control/cam_comp.F90 | 5 ++ src/data/generate_registry_data.py | 11 +-- src/data/registry.xml | 2 +- src/physics/utils/phys_comp.F90 | 45 ++++++---- test/unit/sample_files/reg_bad_xml.xml | 110 +++++++++++++++++++++++++ test/unit/test_registry.py | 20 +++++ 8 files changed, 168 insertions(+), 29 deletions(-) create mode 100644 test/unit/sample_files/reg_bad_xml.xml diff --git a/.gitmodules b/.gitmodules index 94610494..107bf2fd 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,7 +1,7 @@ [submodule "ccpp-framework"] path = ccpp_framework url = https://github.com/NCAR/ccpp-framework - fxtag = 2024-07-19-dev + fxtag = 2024-10-31-dev fxrequired = AlwaysRequired fxDONOTUSEurl = https://github.com/NCAR/ccpp-framework [submodule "history"] diff --git a/ccpp_framework b/ccpp_framework index 5f338ddf..76dbf36d 160000 --- a/ccpp_framework +++ b/ccpp_framework @@ -1 +1 @@ -Subproject commit 5f338ddf02638c06548e54e0a218d154b34faff9 +Subproject commit 76dbf36ddccbe1cf04fa9de04b38338c26b648aa diff --git a/src/control/cam_comp.F90 b/src/control/cam_comp.F90 index 538bd641..3daf0168 100644 --- a/src/control/cam_comp.F90 +++ b/src/control/cam_comp.F90 @@ -86,6 +86,7 @@ subroutine cam_init(caseid, ctitle, model_doi_url, & use cam_initfiles, only: cam_initfiles_open use dyn_grid, only: model_grid_init use phys_comp, only: phys_init + use phys_comp, only: phys_register use dyn_comp, only: dyn_init ! use cam_restart, only: cam_read_restart use camsrfexch, only: hub2atm_alloc, atm2hub_alloc @@ -186,6 +187,10 @@ subroutine cam_init(caseid, ctitle, model_doi_url, & ! Open initial or restart file, and topo file if specified. call cam_initfiles_open() + ! Call CCPP physics register phase (must happen before + ! cam_register_constituents) + call phys_register() + ! Initialize constituent information ! This will set the total number of constituents and the ! number of advected constituents. diff --git a/src/data/generate_registry_data.py b/src/data/generate_registry_data.py index 187cf7be..0b212a9f 100755 --- a/src/data/generate_registry_data.py +++ b/src/data/generate_registry_data.py @@ -1783,16 +1783,7 @@ def gen_registry(registry_file, dycore, outdir, indent, logger, schema_path=schema_dir, error_on_noxmllint=error_on_no_validate) except CCPPError as ccpperr: - cemsg = f"{ccpperr}".split('\n', maxsplit=1)[0] - if cemsg[0:12] == 'Execution of': - xstart = cemsg.find("'") - if xstart >= 0: - xend = cemsg[xstart + 1:].find("'") + xstart + 1 - emsg += '\n' + cemsg[xstart + 1:xend] - # end if (else, just keep original message) - elif cemsg[0:18] == 'validate_xml_file:': - emsg += "\n" + cemsg - # end if + emsg += f"\n{ccpperr}" file_ok = False # end try if not file_ok: diff --git a/src/data/registry.xml b/src/data/registry.xml index 41b31416..4b54088a 100644 --- a/src/data/registry.xml +++ b/src/data/registry.xml @@ -154,7 +154,7 @@ allocatable="pointer"> inverse exner function w.r.t. surface pressure (ps/p)^(R/cp) horizontal_dimension vertical_layer_dimension - exner state_exner + exner state_exner inverse_exner_function_wrt_surface_pressure + + + + + + + + Number of horizontal columns + 0 + + + Number of vertical layers + 0 + + + 1 + + + 2 + + + horizontal_dimension + lat + + + horizontal_dimension + lon + + + horizontal_dimension vertical_layer_dimension + u_wind + + + horizontal_dimension vertical_layer_dimension + v_wind + + + 42 + + + stand_var + + + Composition-dependent ratio of dry air gas constant to specific heat at constant pressure + horizontal_dimension vertical_layer_dimension + 1 + rair/cpair - rair * 2 + + + horizontal_dimension vertical_layer_dimension + number_of_constituents + + Q Q_snapshot + + + CLDLIQ CLDLIQ_snapshot + + + + horizontal_dimension + vertical_layer_dimension + + + x_wind + y_wind + + + + latitude + longitude + model_wind + constituent_mixing_ratio + + + Physics state variables updated by dynamical core + + + $SRCROOT/test/unit/sample_files/ref_pres.meta + diff --git a/test/unit/test_registry.py b/test/unit/test_registry.py index 98d0a232..0b6bb6c6 100644 --- a/test/unit/test_registry.py +++ b/test/unit/test_registry.py @@ -42,6 +42,7 @@ from generate_registry_data import gen_registry from generate_registry_data import metadata_file_to_files, TypeRegistry from framework_env import CCPPFrameworkEnv +from parse_source import CCPPError # pylint: enable=wrong-import-position ############################################################################### @@ -116,6 +117,25 @@ def test_good_simple_registry(self): self.assertTrue(filecmp.cmp(out_source, in_source, shallow=False), msg=amsg) + def test_bad_registry_xml(self): + """Test that the full error messages from xmllint is returned""" + # Setup test + filename = os.path.join(_SAMPLE_FILES_DIR, "reg_bad_xml.xml") + out_name = "physics_types_bad" + # Try to generate the registry + with self.assertRaises(CCPPError) as cerr: + retcode, files, _ = gen_registry(filename, 'se', _TMP_DIR, 2, + _SRC_MOD_DIR, _CAM_ROOT, + loglevel=logging.ERROR, + error_on_no_validate=True) + # end with + expected_error = "reg_bad_xml.xml:32: element ic_file_input_name: Schemas validity error : Element 'ic_file_input_name': This element is not expected. Expected is one of ( initial_value, ic_file_input_names )." + split_exception = str(cerr.exception).split('\n') + amsg = f"Test failure: exception raised is {len(split_exception)} lines long and is expected to be 4" + self.assertEqual(len(split_exception), 4, msg=amsg) + # Check that the full xmllint message was returned + self.assertTrue(split_exception[2].endswith(expected_error)) + def test_good_ddt_registry(self): """Test code and metadata generation from a good registry with a DDT. Check that generate_registry_data.py generates good