Skip to content

Commit 4ca1f31

Browse files
authored
Check initial value attribute(s) (#376)
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): Adds logic to check the initial_value `dycore` attribute, if present, against the dycore. Hopefully also will be "easy" to add additional attributes in the future. The logic is: - Will pick the initial_value entry with the "most" attribute matches (which, right now, just means it'll pick the default if the dycore doesn't match other entries or if there are no dycore-specific entries) - Will error if there is a "tie" (multiple entries have the same number of attribute matches) Describe any changes made to build system: **M src/data/generate_registry_data.py** - check dycore attribute for initial_value, if present - also update "dycore" to "dyn" **M src/data/registry.xml** - update "dycore" to "dyn" **M src/data/registry_v1_0.xsd** - update "dycore" to "dyn" 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 test/unit/python/sample_files/*.xml** - add additional options for dycore initial_value attribute **M test/unit/python/sample_files/*.F90** - update expected files corresponding to .xml updates **M test/unit/python/test_registry.py** - change "dycore" to "dyn" 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: CAM-SIMA date used for the baseline comparison tests if different than latest:
1 parent fd841b5 commit 4ca1f31

File tree

9 files changed

+109
-26
lines changed

9 files changed

+109
-26
lines changed

src/data/generate_registry_data.py

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class VarBase:
141141
__pointer_type_str = "pointer"
142142

143143
def __init__(self, elem_node, local_name, dimensions, known_types,
144-
type_default, units_default="", kind_default='',
144+
type_default, units_default="", kind_default='', dycore='',
145145
protected=False, index_name='', local_index_name='',
146146
local_index_name_str='', alloc_default='none',
147147
tstep_init_default=False):
@@ -154,6 +154,7 @@ def __init__(self, elem_node, local_name, dimensions, known_types,
154154
self.__standard_name = elem_node.get('standard_name')
155155
self.__long_name = ''
156156
self.__initial_value = ''
157+
self.__initial_value_match = 0
157158
self.__initial_val_vars = set()
158159
self.__ic_names = None
159160
self.__elements = []
@@ -183,7 +184,30 @@ def __init__(self, elem_node, local_name, dimensions, known_types,
183184
if attrib.tag == 'long_name':
184185
self.__long_name = attrib.text
185186
elif attrib.tag == 'initial_value':
186-
self.__initial_value = attrib.text
187+
# Figure out if we should use this initial_value
188+
# If the number of matching attributes is greater than the
189+
# default or a previous match, pick this one
190+
matches = 0
191+
if not attrib.keys():
192+
self.__initial_value = attrib.text
193+
# end if (no attributes, this is the default)
194+
for att in attrib.keys():
195+
# Check each attribute (for now, this is only the dycore)
196+
if att == 'dyn':
197+
dycore_list = attrib.get('dyn').lower().split(',')
198+
if dycore in dycore_list:
199+
matches += 1
200+
# end if (dycore matches)
201+
# end if (check the dycore)
202+
# end for
203+
if matches == self.__initial_value_match and matches != 0:
204+
emsg = f"Unclear which initial_value to use for {local_name}. There are at least two configurations with {matches} matching attributes"
205+
raise CCPPError(emsg)
206+
elif matches > self.__initial_value_match:
207+
# Use this initial_value (for now)
208+
self.__initial_value_match = matches
209+
self.__initial_value = attrib.text
210+
# end if (number of matches)
187211
elif attrib.tag == 'ic_file_input_names':
188212
#Separate out string into list:
189213
self.__ic_names = [x.strip() for x in attrib.text.split(' ') if x]
@@ -488,16 +512,16 @@ class Variable(VarBase):
488512
###############################################################################
489513
# pylint: disable=too-many-instance-attributes
490514
"""Registry variable
491-
>>> Variable(ET.fromstring('<variable kind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1"><dimensions>ccpp_constant_one:horizontal_dimension:two</dimensions></variable>'), TypeRegistry(), VarDict("foo", "module", None), None) #doctest: +IGNORE_EXCEPTION_DETAIL
515+
>>> Variable(ET.fromstring('<variable kind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1"><dimensions>ccpp_constant_one:horizontal_dimension:two</dimensions></variable>'), TypeRegistry(), VarDict("foo", "module", None), 'se', None) #doctest: +IGNORE_EXCEPTION_DETAIL
492516
Traceback (most recent call last):
493517
CCPPError: Illegal dimension string, ccpp_constant_one:horizontal_dimension:two, in u, step not allowed.
494-
>>> Variable(ET.fromstring('<variable kind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1"><dims>horizontal_dimension</dims></variable>'), TypeRegistry(), VarDict("foo", "module", None), None) #doctest: +IGNORE_EXCEPTION_DETAIL
518+
>>> Variable(ET.fromstring('<variable kind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1"><dims>horizontal_dimension</dims></variable>'), TypeRegistry(), VarDict("foo", "module", None), 'se', None) #doctest: +IGNORE_EXCEPTION_DETAIL
495519
Traceback (most recent call last):
496520
CCPPError: Unknown Variable content, dims
497-
>>> Variable(ET.fromstring('<variable kkind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1"></variable>'), TypeRegistry(), VarDict("foo", "module", None), None) #doctest: +IGNORE_EXCEPTION_DETAIL
521+
>>> Variable(ET.fromstring('<variable kkind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1"></variable>'), TypeRegistry(), VarDict("foo", "module", None), 'se', None) #doctest: +IGNORE_EXCEPTION_DETAIL
498522
Traceback (most recent call last):
499523
CCPPError: Bad variable attribute, 'kkind', for 'u'
500-
>>> Variable(ET.fromstring('<variable kind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1" allocatable="target"><dimensions>horizontal_dimension vertical_dimension</dimensions></variable>'), TypeRegistry(), VarDict("foo", "module", None), None) #doctest: +IGNORE_EXCEPTION_DETAIL
524+
>>> Variable(ET.fromstring('<variable kind="kind_phys" local_name="u" standard_name="east_wind" type="real" units="m s-1" allocatable="target"><dimensions>horizontal_dimension vertical_dimension</dimensions></variable>'), TypeRegistry(), VarDict("foo", "module", None), 'se', None) #doctest: +IGNORE_EXCEPTION_DETAIL
501525
Traceback (most recent call last):
502526
CCPPError: Dimension, 'vertical_dimension', not found for 'u'
503527
"""
@@ -511,7 +535,7 @@ class Variable(VarBase):
511535
"phys_timestep_init_zero", "standard_name",
512536
"type", "units", "version"]
513537

514-
def __init__(self, var_node, known_types, vdict, logger):
538+
def __init__(self, var_node, known_types, vdict, dycore, logger):
515539
# pylint: disable=too-many-locals
516540
"""Initialize a Variable from registry XML"""
517541
local_name = var_node.get('local_name')
@@ -588,7 +612,7 @@ def __init__(self, var_node, known_types, vdict, logger):
588612
# Initialize the base class
589613
super().__init__(var_node, local_name,
590614
my_dimensions, known_types, ttype,
591-
protected=protected)
615+
dycore=dycore, protected=protected)
592616

593617
for attrib in var_node:
594618
# Second pass, only process array elements
@@ -1090,7 +1114,7 @@ def __init__(self, ddt_node, known_types, var_dict, dycore):
10901114
varname = attrib.text
10911115
include_var = True
10921116
attrib_dycores = [x.strip().lower() for x in
1093-
attrib.get('dycore', default="").split(',')
1117+
attrib.get('dyn', default="").split(',')
10941118
if x]
10951119
if attrib_dycores and (dycore not in attrib_dycores):
10961120
include_var = False
@@ -1226,6 +1250,7 @@ def __init__(self, file_node, known_types, dycore,
12261250
self.__use_statements = []
12271251
self.__generate_code = gen_code
12281252
self.__file_path = file_path
1253+
self.__dycore = dycore
12291254
for obj in file_node:
12301255
if obj.tag in ['variable', 'array']:
12311256
self.add_variable(obj, logger)
@@ -1252,7 +1277,7 @@ def add_variable(self, var_node, logger):
12521277
"""Create a Variable from <var_node> and add to this File's
12531278
variable dictionary"""
12541279
newvar = Variable(var_node, self.__known_types, self.__var_dict,
1255-
logger)
1280+
self.__dycore, logger)
12561281
self.__var_dict.add_variable(newvar)
12571282

12581283
def add_ddt(self, newddt, logger=None):

src/data/registry.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@
349349
units="flag" type="logical">
350350
<long_name>flag indicating if dynamical core energy is not consistent with CAM physics and to perform adjustment of temperature and temperature tendency</long_name>
351351
<initial_value>.false.</initial_value>
352-
<initial_value dycore="SE,MPAS">.true.</initial_value>
352+
<initial_value dyn="SE,MPAS">.true.</initial_value>
353353
</variable>
354354
<!-- Timestep properties -->
355355
<variable local_name="is_first_timestep"
@@ -392,7 +392,7 @@
392392
standard_name="dycore_calculates_geopotential_using_logarithms"
393393
units="flag" type="logical" access="protected">
394394
<initial_value>.false.</initial_value>
395-
<initial_value dycore="SE,FV">.true.</initial_value>
395+
<initial_value dyn="SE,FV">.true.</initial_value>
396396
</variable>
397397
<ddt type="physics_state">
398398
<data>air_temperature</data>

src/data/registry_v1_0.xsd

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696

9797
<xs:attribute name="access" type="access_type"/>
9898
<xs:attribute name="allocatable" type="allocation_type"/>
99-
<xs:attribute name="dycore" type="dycore_type"/>
99+
<xs:attribute name="dyn" type="dycore_type"/>
100100
<xs:attribute name="extends" type="fortran_id_type"/>
101101
<xs:attribute name="kind" type="fortran_id_type"/>
102102
<xs:attribute name="local_name" type="fortran_id_type"/>
@@ -130,7 +130,10 @@
130130
<xs:complexType name="initial_value">
131131
<xs:simpleContent>
132132
<xs:extension base="xs:string">
133-
<xs:attribute ref="dycore" use="optional" default=""/>
133+
<!--- If adding a new attribute, also modify generate_registry_data.py
134+
to check the attribute
135+
-->
136+
<xs:attribute ref="dyn" use="optional" default=""/>
134137
</xs:extension>
135138
</xs:simpleContent>
136139
</xs:complexType>
@@ -192,7 +195,7 @@
192195
<xs:complexType name="data_type">
193196
<xs:simpleContent>
194197
<xs:extension base="standard_name_type">
195-
<xs:attribute ref="dycore" use="optional" default=""/>
198+
<xs:attribute ref="dyn" use="optional" default=""/>
196199
</xs:extension>
197200
</xs:simpleContent>
198201
</xs:complexType>

test/unit/python/sample_files/physics_types_complete.F90

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ module physics_types_complete
2929
!! \htmlinclude physics_base.html
3030
type, bind(C) :: physics_base
3131
! ncol: Number of horizontal columns
32-
integer :: ncol = 0
32+
integer :: ncol = 1
3333
! pver: Number of vertical layers
3434
integer :: pver = 0
3535
end type physics_base
@@ -197,7 +197,7 @@ subroutine allocate_physics_types_complete_fields(horizontal_dimension,
197197
phys_state%q(:,:,ix_cld_liq) = nan
198198
end if
199199
if (set_init_val) then
200-
phys_state%ncol = 0
200+
phys_state%ncol = 1
201201
end if
202202
if (set_init_val) then
203203
phys_state%pver = 0

test/unit/python/sample_files/reg_good_complete.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
units="count" type="integer" access="protected">
1010
<long_name>Number of horizontal columns</long_name>
1111
<initial_value>0</initial_value>
12+
<initial_value dyn='SE'>1</initial_value>
13+
<initial_value dyn='FV,MPAS'>2</initial_value>
1214
</variable>
1315
<variable local_name="pver" standard_name="vertical_layer_dimension"
1416
units="count" type="integer" access="protected">

test/unit/python/sample_files/reg_good_ddt.xml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@
3131
<initial_value>rair/cpair</initial_value>
3232
</variable>
3333
<ddt type="physics_state">
34-
<data dycore="FV,EUL,SE">horizontal_dimension</data>
35-
<data dycore="SE">latitude</data>
36-
<data dycore="FV">longitude</data>
34+
<data dyn="FV,EUL,SE">horizontal_dimension</data>
35+
<data dyn="SE">latitude</data>
36+
<data dyn="FV">longitude</data>
3737
</ddt>
3838
<variable local_name="phys_state"
3939
standard_name="physics_state_due_to_dynamics"

test/unit/python/sample_files/reg_good_mf.xml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@
3131
<initial_value>rair/cpair</initial_value>
3232
</variable>
3333
<ddt type="physics_state">
34-
<data dycore="FV,EUL,SE">horizontal_dimension</data>
35-
<data dycore="SE">latitude</data>
36-
<data dycore="FV">longitude</data>
34+
<data dyn="FV,EUL,SE">horizontal_dimension</data>
35+
<data dyn="SE">latitude</data>
36+
<data dyn="FV">longitude</data>
3737
</ddt>
3838
<variable local_name="phys_state"
3939
standard_name="physics_state_due_to_dynamics"

test/unit/python/sample_files/reg_good_simple.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
units="count" type="integer" access="protected">
88
<long_name>Number of horizontal columns</long_name>
99
<initial_value>0</initial_value>
10+
<initial_value dyn="SE">1</initial_value>
1011
</variable>
1112
<variable local_name="latitude" standard_name="latitude"
1213
units="radians" type="real" kind="kind_phys"

test/unit/python/test_registry.py

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ def setUpClass(cls):
8585
def test_good_simple_registry(self):
8686
"""Test that a good registry with only variables validates.
8787
Check that generate_registry_data.py generates good
88-
Fortran and metadata files"""
88+
Fortran and metadata files. Expectation is that the initial
89+
value for ncol will be set to the default of 0 because there is no
90+
dycore match"""
8991
# Setup test
9092
filename = os.path.join(_SAMPLE_FILES_DIR, "reg_good_simple.xml")
9193
out_source_name = "physics_types_simple"
@@ -607,6 +609,56 @@ def test_bad_registry_version(self):
607609
self.assertFalse(os.path.exists(out_meta))
608610
self.assertFalse(os.path.exists(out_source))
609611

612+
def test_registry_no_clear_initial_value(self):
613+
"""
614+
Check that generate_registry_data.py throws the correct
615+
error message when there is no clear "winner"
616+
between the initial_value entries
617+
"""
618+
619+
# Setup test
620+
infilename = os.path.join(_SAMPLE_FILES_DIR, "reg_good_simple.xml")
621+
tree, root = read_xml_file(infilename)
622+
623+
# Open base XML file:
624+
base_tree = ET.parse(infilename)
625+
base_root = base_tree.getroot()
626+
627+
# Write new namelist entry with "bad" value:
628+
initial_value_tie_string = \
629+
"""
630+
<variable local_name="myvar" standard_name="variable_with_unclear_initial_value"
631+
units="count" type="integer" access="protected">
632+
<long_name>Number of horizontal columns</long_name>
633+
<initial_value>2</initial_value>
634+
<initial_value dyn="SE">0</initial_value>
635+
<initial_value dyn="SE">1</initial_value>
636+
</variable>
637+
"""
638+
639+
initial_value_tie_entry = ET.fromstring(initial_value_tie_string)
640+
641+
# Add new namelist entry back to original namelist XML tree:
642+
base_root[0].append(initial_value_tie_entry)
643+
644+
# Write out new, temporary XML namelist file for testing:
645+
xml_test_fil = os.path.join(_TMP_DIR, "test_registry_initial_value_tie.xml")
646+
base_tree.write(xml_test_fil, encoding="utf-8", xml_declaration=True)
647+
filename = os.path.join(_TMP_DIR, "test_registry_initial_value_tie.xml")
648+
649+
# Attempt to generate registry:
650+
with self.assertRaises(CCPPError) as cerr:
651+
retcode, files, _ = gen_registry(filename, 'se', _TMP_DIR, 2,
652+
_SRC_MOD_DIR, _CAM_ROOT,
653+
loglevel=logging.ERROR,
654+
error_on_no_validate=True)
655+
656+
# Check exception message
657+
emsg = "Unclear which initial_value to use for myvar. "
658+
emsg += "There are at least two configurations with 1 matching attributes"
659+
660+
self.assertEqual(emsg, str(cerr.exception))
661+
610662
def test_missing_standard_name(self):
611663
"""Test a registry with a missing standard name.
612664
Check that it does not validate and does not generate any
@@ -812,10 +864,10 @@ def test_duplicate_type(self):
812864
new_ddt = ET.SubElement(obj, "ddt")
813865
new_ddt.set("type", dtype)
814866
data_elem = ET.SubElement(new_ddt, "data")
815-
data_elem.set("dycore", "EUL")
867+
data_elem.set("dyn", "EUL")
816868
data_elem.text = 'latitude'
817869
data_elem = ET.SubElement(new_ddt, "data")
818-
data_elem.set("dycore", "EUL")
870+
data_elem.set("dyn", "EUL")
819871
data_elem.text = 'longitude'
820872
break
821873
# End if

0 commit comments

Comments
 (0)