Skip to content

Commit

Permalink
Add new register phase (#319)
Browse files Browse the repository at this point in the history
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](NCAR/ccpp-framework#582)
2. Uses the full error message returned from the exception handled when
xmllint is called (updated in framework PR
[#586](NCAR/ccpp-framework#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...<your_branch_name>`)

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 <courtneyp@izumi.cgd.ucar.edu>
  • Loading branch information
peverwhee and Courtney Peverley authored Nov 6, 2024
1 parent e69640a commit 455003c
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -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"]
Expand Down
2 changes: 1 addition & 1 deletion ccpp_framework
Submodule ccpp_framework updated 46 files
+1 −1 .github/workflows/capgen_unit_tests.yaml
+3 −49 scripts/ccpp_capgen.py
+0 −65 scripts/ccpp_datafile.py
+4 −1 scripts/ccpp_state_machine.py
+15 −12 scripts/ccpp_suite.py
+1 −1 scripts/ccpp_track_variables.py
+13 −49 scripts/constituents.py
+86 −10 scripts/host_cap.py
+1 −10 scripts/metadata_table.py
+1 −1 scripts/parse_tools/parse_checkers.py
+26 −3 scripts/parse_tools/xml_tools.py
+22 −1 scripts/suite_objects.py
+7 −0 scripts/var_props.py
+3 −0 src/ccpp_constituent_prop_mod.F90
+11 −0 src/ccpp_constituent_prop_mod.meta
+45 −0 test/advection_test/CMakeLists.txt
+28 −26 test/advection_test/cld_ice.F90
+25 −1 test/advection_test/cld_ice.meta
+27 −21 test/advection_test/cld_liq.F90
+25 −2 test/advection_test/cld_liq.meta
+9 −0 test/advection_test/cld_suite_error.xml
+3 −0 test/advection_test/cld_suite_files_error.txt
+41 −0 test/advection_test/dlc_liq.F90
+29 −0 test/advection_test/dlc_liq.meta
+4 −2 test/advection_test/run_test
+32 −2 test/advection_test/test_host.F90
+5 −3 test/advection_test/test_reports.py
+2 −0 test/capgen_test/run_test
+22 −0 test/capgen_test/temp_adjust.F90
+24 −0 test/capgen_test/temp_adjust.meta
+10 −0 test/capgen_test/test_host.F90
+1 −0 test/capgen_test/test_host_mod.F90
+7 −1 test/capgen_test/test_host_mod.meta
+1 −0 test/capgen_test/test_reports.py
+0 −1 test/unit_tests/sample_files/test_host.meta
+0 −96 test/unit_tests/sample_scheme_files/duplicate_dyn_const.F90
+0 −104 test/unit_tests/sample_scheme_files/duplicate_dyn_const.meta
+0 −75 test/unit_tests/sample_scheme_files/dyn_const_not_present.F90
+0 −104 test/unit_tests/sample_scheme_files/dyn_const_not_present.meta
+0 −100 test/unit_tests/sample_scheme_files/dyn_const_not_present_nested.F90
+0 −104 test/unit_tests/sample_scheme_files/dyn_const_not_present_nested.meta
+21 −21 test/unit_tests/sample_scheme_files/temp_adjust.F90
+32 −1 test/unit_tests/sample_scheme_files/temp_adjust.meta
+5 −38 test/unit_tests/test_metadata_scheme_file.py
+0 −3 test/unit_tests/test_metadata_table.py
+1 −1 test/var_compatibility_test/test_host.meta
5 changes: 5 additions & 0 deletions src/control/cam_comp.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 1 addition & 10 deletions src/data/generate_registry_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/data/registry.xml
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@
allocatable="pointer">
<long_name>inverse exner function w.r.t. surface pressure (ps/p)^(R/cp)</long_name>
<dimensions>horizontal_dimension vertical_layer_dimension</dimensions>
<ic_file_input_names>exner state_exner</ic_file_input_names>
<ic_file_input_names>exner state_exner inverse_exner_function_wrt_surface_pressure</ic_file_input_names>
</variable>
<variable local_name="zm"
standard_name="geopotential_height_wrt_surface"
Expand Down
45 changes: 29 additions & 16 deletions src/physics/utils/phys_comp.F90
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module phys_comp
private

public :: phys_readnl
public :: phys_register
public :: phys_init
public :: phys_timestep_init
public :: phys_run1
Expand Down Expand Up @@ -129,26 +130,14 @@ subroutine phys_readnl(nlfilename)

end subroutine phys_readnl

subroutine phys_init()
use cam_abortutils, only: endrun
use physics_grid, only: columns_on_task
use vert_coord, only: pver, pverp
use cam_thermo, only: cam_thermo_init
use physics_types, only: allocate_physics_types_fields
use cam_ccpp_cap, only: cam_ccpp_physics_initialize
subroutine phys_register()
use cam_ccpp_cap, only: cam_ccpp_physics_register
use cam_ccpp_cap, only: ccpp_physics_suite_part_list
use cam_abortutils, only: endrun

! Local variables
integer :: i_group

call cam_thermo_init(columns_on_task, pver, pverp)

call allocate_physics_types_fields(columns_on_task, pver, pverp, &
set_init_val_in=.true., reallocate_in=.false.)
call cam_ccpp_physics_initialize(phys_suite_name)
if (errcode /= 0) then
call endrun('cam_ccpp_physics_initialize: '//trim(errmsg))
end if
call ccpp_physics_suite_part_list(phys_suite_name, suite_parts, &
errmsg, errcode)
if (errcode /= 0) then
Expand All @@ -158,11 +147,35 @@ subroutine phys_init()
! Confirm that the suite parts are as expected
do i_group = 1, size(suite_parts)
if (.not. any(suite_parts(i_group) == suite_parts_expect)) then
write(errmsg, *) 'phys_init: SDF suite groups MUST be ', &
write(errmsg, *) 'phys_register: SDF suite groups MUST be ', &
'ONLY `physics_before_coupler` and/or `physics_after_coupler`'
call endrun(errmsg)
end if
end do
! Call CCPP register phase
call cam_ccpp_physics_register(phys_suite_name)
if (errcode /= 0) then
call endrun('cam_ccpp_physics_register: '//trim(errmsg))
end if

end subroutine phys_register

subroutine phys_init()
use cam_abortutils, only: endrun
use physics_grid, only: columns_on_task
use vert_coord, only: pver, pverp
use cam_thermo, only: cam_thermo_init
use physics_types, only: allocate_physics_types_fields
use cam_ccpp_cap, only: cam_ccpp_physics_initialize

call cam_thermo_init(columns_on_task, pver, pverp)

call allocate_physics_types_fields(columns_on_task, pver, pverp, &
set_init_val_in=.true., reallocate_in=.false.)
call cam_ccpp_physics_initialize(phys_suite_name)
if (errcode /= 0) then
call endrun('cam_ccpp_physics_initialize: '//trim(errmsg))
end if

end subroutine phys_init

Expand Down
110 changes: 110 additions & 0 deletions test/unit/sample_files/reg_bad_xml.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
<?xml version="1.0" encoding="UTF-8"?>

<registry name="cam_registry" version="1.0">
<file name="physics_types_complete" type="module">
<use module="ccpp_kinds" reference="kind_phys"/>
<use module="physconst" reference="rair"/>
<use module="physconst" reference="cpair"/>
<variable local_name="ncol" standard_name="horizontal_dimension"
units="count" type="integer" access="protected">
<long_name>Number of horizontal columns</long_name>
<initial_value>0</initial_value>
</variable>
<variable local_name="pver" standard_name="vertical_layer_dimension"
units="count" type="integer" access="protected">
<long_name>Number of vertical layers</long_name>
<initial_value>0</initial_value>
</variable>
<variable local_name="ix_qv"
standard_name="index_of_water_vapor_specific_humidity"
units="count" type="integer">
<initial_value>1</initial_value>
</variable>
<variable local_name="ix_cld_liq"
standard_name="index_of_cloud_liquid_water_mixing_ratio_of_moist_air"
units="count" type="integer">
<initial_value>2</initial_value>
</variable>
<variable local_name="latitude" standard_name="latitude"
units="radians" type="real" kind="kind_phys"
allocatable="pointer" access="protected">
<dimensions>horizontal_dimension</dimensions>
<ic_file_input_name>lat</ic_file_input_name>
</variable>
<variable local_name="longitude" standard_name="longitude"
units="radians" type="real" kind="kind_phys"
allocatable="pointer" access="protected">
<dimensions>horizontal_dimension</dimensions>
<ic_file_input_names>lon</ic_file_input_names>
</variable>
<variable local_name="u" standard_name="x_wind"
units="m s-1" type="real" kind="kind_phys"
allocatable="pointer" access="protected">
<dimensions>horizontal_dimension vertical_layer_dimension</dimensions>
<ic_file_input_names>u_wind</ic_file_input_names>
</variable>
<variable local_name="v" standard_name="y_wind"
units="m s-1" type="real" kind="kind_phys"
allocatable="pointer" access="protected">
<dimensions>horizontal_dimension vertical_layer_dimension</dimensions>
<ic_file_input_names>v_wind</ic_file_input_names>
</variable>
<variable local_name="param_val_var"
standard_name="made_up_param_variable"
units="count" type="integer" allocatable="parameter">
<initial_value>42</initial_value>
</variable>
<variable local_name="standard_var"
standard_name="standard_non_ddt_variable"
units="K" type="real" phys_timestep_init_zero="true">
<ic_file_input_names>stand_var</ic_file_input_names>
</variable>
<variable local_name="cappav"
standard_name="composition_dependent_ratio_of_dry_air_gas_constant_to_specific_heat_at_constant_pressure"
units="1" type="real" kind="kind_phys"
allocatable="allocatable">
<long_name>Composition-dependent ratio of dry air gas constant to specific heat at constant pressure</long_name>
<dimensions>horizontal_dimension vertical_layer_dimension</dimensions>
<initial_value>1 + rair/cpair - rair * 2</initial_value>
</variable>
<array local_name="q" standard_name="constituent_mixing_ratio"
units="kg kg-1"
type="real" kind="kind_phys"
allocatable="pointer">
<dimensions>horizontal_dimension vertical_layer_dimension
number_of_constituents</dimensions>
<element standard_name="water_vapor_specific_humidity"
index_name="index_of_water_vapor_specific_humidity"
index_pos="number_of_constituents">
<ic_file_input_names>Q Q_snapshot</ic_file_input_names>
</element>
<element standard_name="cloud_liquid_water_mixing_ratio_of_moist_air"
index_name="index_of_cloud_liquid_water_mixing_ratio_of_moist_air"
index_pos="number_of_constituents">
<ic_file_input_names>CLDLIQ CLDLIQ_snapshot</ic_file_input_names>
</element>
</array>
<ddt type="physics_base" bindC="true">
<data>horizontal_dimension</data>
<data>vertical_layer_dimension</data>
</ddt>
<ddt type="model_wind">
<data>x_wind</data>
<data>y_wind</data>
</ddt>
<variable local_name="wind" standard_name="model_wind"
units="None" type="model_wind" />
<ddt type="physics_state" extends="physics_base">
<data>latitude</data>
<data>longitude</data>
<data>model_wind</data>
<data>constituent_mixing_ratio</data>
</ddt>
<variable local_name="phys_state"
standard_name="physics_state_due_to_dynamics"
units="None" type="physics_state" phys_timestep_init_zero="true">
<long_name>Physics state variables updated by dynamical core</long_name>
</variable>
</file>
<metadata_file>$SRCROOT/test/unit/sample_files/ref_pres.meta</metadata_file>
</registry>
20 changes: 20 additions & 0 deletions test/unit/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

###############################################################################
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 455003c

Please sign in to comment.