EAMxx: fix doctests and corner-cases in atm_manip

bartgol · bartgol · commit fa3ff46c4a93 · 2024-09-26T14:56:31.000-06:00
diff --git a/components/eamxx/cime_config/eamxx_buildnml.py b/components/eamxx/cime_config/eamxx_buildnml.py
@@ -126,7 +126,7 @@ def perform_consistency_checks(case, xml):
     >>> case = MockCase({'ATM_NCPL':'24', 'REST_N':2, 'REST_OPTION':'nsteps'})
     >>> perform_consistency_checks(case,xml)
     Traceback (most recent call last):
-    CIME.utils.CIMEError: ERROR: rrtmgp::rad_frequency incompatible with restart frequency.
+    CIME.utils.CIMEError: ERROR: rrtmgp::rad_frequency (3 steps) incompatible with restart frequency (2 steps).
      Please, ensure restart happens on a step when rad is ON
     >>> case = MockCase({'ATM_NCPL':'24', 'REST_N':10800, 'REST_OPTION':'nseconds'})
     >>> perform_consistency_checks(case,xml)
@@ -970,7 +970,7 @@ def create_input_data_list_file(case,caseroot):
                             permissions = stat.filemode(file_stat.st_mode)
 
                         except Exception as e:
-                            raise RuntimeError(f"Error retrieving file info for '{file_path}': {e}")
+                            raise RuntimeError(f"Error retrieving file info for '{file_path}': {e}") from e
 
                         curr_user = getpass.getuser()
                         user_info = pwd.getpwnam(curr_user)
diff --git a/components/eamxx/scripts/atm_manip.py b/components/eamxx/scripts/atm_manip.py
@@ -89,11 +89,11 @@ def get_xml_nodes(xml_root, name):
     >>> ################ INVALID SYNTAX #######################
     >>> get_xml_nodes(tree,'sub::::prop1')
     Traceback (most recent call last):
-    SystemExit: ERROR: Invalid xml node name format, 'sub::::prop1' contains ::::
+    SystemExit: ERROR: Invalid xml node name format, 'sub::::prop1' contains '::::'
     >>> ################ VALID USAGE #######################
     >>> get_xml_nodes(tree,'invalid::prop1')
     []
-    >>> [item.text for item in get_xml_nodes(tree,'prop1')]
+    >>> [item.text for item in get_xml_nodes(tree,'ANY::prop1')]
     ['one', 'two']
     >>> [item.text for item in get_xml_nodes(tree,'::prop1')]
     ['one']
@@ -102,10 +102,10 @@ def get_xml_nodes(xml_root, name):
     >>> item = get_xml_nodes(tree,'prop2')[0]
     >>> parent_map = create_parent_map(tree)
     >>> [p.tag for p in get_parents(item, parent_map)]
-    ['root', 'sub']
+    ['sub']
     """
     expect('::::' not in name,
-            "Badly formatted node name: found '::::'")
+            f"Invalid xml node name format, '{name}' contains '::::'")
 
     tokens = name.split("::")
     expect (tokens[-1] != '', "Input query string ends with '::'. It should end with an actual node name")
@@ -156,7 +156,8 @@ def get_xml_nodes(xml_root, name):
         error_str = f"{name} is ambiguous. Use ANY in the node path to allow multiple matches. Matches:\n"
         for node in result:
             parents = get_parents(node, parent_map)
-            name = "::".join(e.tag for e in parents) + "::" + node.tag
+            name = "::".join(e.tag for e in parents)
+            name = node.tag if name=="" else name + "::" + node.tag
             error_str += "  " + name + "\n"
 
         expect(False, error_str)
@@ -393,8 +394,8 @@ def atm_config_chg_impl(xml_root, change):
     >>> atm_config_chg_impl(tree,'prop1=three')
     Traceback (most recent call last):
     SystemExit: ERROR: prop1 is ambiguous. Use ANY in the node path to allow multiple matches. Matches:
-      root::prop1
-      root::sub::prop1
+      prop1
+      sub::prop1
     <BLANKLINE>
     >>> ################ VALID USAGE #######################
     >>> atm_config_chg_impl(tree,'::prop1=two')
@@ -405,7 +406,7 @@ def atm_config_chg_impl(xml_root, change):
     True
     >>> atm_config_chg_impl(tree,'ANY::prop1=three')
     True
-    >>> [item.text for item in get_xml_nodes(tree,'prop1')]
+    >>> [item.text for item in get_xml_nodes(tree,'ANY::prop1')]
     ['three', 'three']
     >>> ################ TEST APPEND += #################
     >>> atm_config_chg_impl(tree,'a+=4')
@@ -465,14 +466,49 @@ def get_parents(elem, parent_map):
     be the furthest ancestor, last item will be direct parent)
     """
     results = []
-    if elem in parent_map and parent_map[elem] is not None:
+    if not is_root(elem,parent_map):
         parent = parent_map[elem]
         results = get_parents(parent, parent_map)
-        if parent_map[parent] is not None:
+        if not is_root(parent,parent_map):
             results.append(parent)
 
     return results
 
+###############################################################################
+def is_anchestor_of(parent,child,parent_map):
+###############################################################################
+    """
+    >>> xml = '''
+    ... <root>
+    ...     <prop1>one</prop1>
+    ...     <sub>
+    ...         <prop1>two</prop1>
+    ...         <prop2 type="integer" valid_values="1,2">2</prop2>
+    ...     </sub>
+    ... </root>
+    ... '''
+    >>> import xml.etree.ElementTree as ET
+    >>> tree = ET.fromstring(xml)
+    >>> parent_map = create_parent_map(tree)
+    >>> sub = get_xml_nodes(tree,'sub')[0]
+    >>> sub_prop1 = get_xml_nodes(tree,'sub::prop1')[0]
+    >>> is_anchestor_of(sub,sub_prop1,parent_map)
+    True
+    >>> is_anchestor_of(sub_prop1,sub,parent_map)
+    False
+    """
+    curr = child
+    while curr is not None:
+        if curr is parent:
+            return True
+        curr = parent_map[curr]
+    return False
+
+###############################################################################
+def is_root (node,parent_map):
+###############################################################################
+    return parent_map[node] is None
+
 ###############################################################################
 def print_var_impl(node,parent_map,full,dtype,value,valid_values,print_style="invalid",indent=""):
 ###############################################################################
@@ -482,21 +518,23 @@ def print_var_impl(node,parent_map,full,dtype,value,valid_values,print_style="in
         # This is not a leaf, so print all nested nodes.
         for child in node:
             # Since prints are nicely nested, use 'node-name' as print style
-            print_var_impl(child,{},full,dtype,value,valid_values,'node-name',indent+"  ")
+            print_var_impl(child,{},full,dtype,value,valid_values,'node-name',indent+"    ")
         return
 
-    #  print (f"printing leaf={node.tag}, len(parent_map)={len(parent_map)}")
     expect (print_style in ["node-name","full-scope","parent-scope"],
             f"Invalid print_style '{print_style}' for print_var_impl. Use 'full' or 'short'.")
 
     if print_style=="node-name":
         # Just the inner most name
         name = node.tag
     elif print_style=="parent-scope":
-        name = parent_map[node].tag + "::" + node.tag
+        parent = parent_map[node]
+        name = node.tag if is_root(parent,parent_map) else parent.tag + "::" + node.tag
     else:
         parents = get_parents(node, parent_map)
-        name = "::".join(e.tag for e in parents) + "::" + node.tag
+        name = "::".join(e.tag for e in parents)
+        name += "::" if parents else ""
+        name += node.tag
 
     if full:
         expect ("type" in node.attrib.keys(),
@@ -563,7 +601,19 @@ def print_var(xml_root,parent_map,var,full,dtype,value,valid_values,print_style=
     # Get matches
     matches = get_xml_nodes(xml_root,var)
 
-    for node in matches:
+    # If ANY is in the var name, we may hit the case where one of the matches
+    # is a parent of another match. In this case, we want to get rid of 
+    unique_matches = []
+    for i in matches:
+        add_this = True
+        for j in matches:
+            if i is not j and is_anchestor_of(j,i,parent_map):
+                add_this = False
+            
+        if add_this:
+            unique_matches.append(i)
+
+    for node in unique_matches:
         print_var_impl(node,parent_map,full,dtype,value,valid_values,print_style,indent)
 
 ###############################################################################
@@ -584,23 +634,22 @@ def atm_query_impl(xml_root,variables,listall=False,full=False,value=False,
     >>> tree = ET.fromstring(xml)
     >>> vars = ['prop2','::prop1']
     >>> success = atm_query_impl(tree, vars)
-        root::sub::prop2: 2
-        root::prop1: one
+        sub::prop2: 2
+        prop1: one
     >>> success = atm_query_impl(tree, [], listall=True, valid_values=True)
-        root
+        prop1: <valid values not provided>
+        sub:
             prop1: <valid values not provided>
-            sub
-                prop1: <valid values not provided>
-                prop2: ['1', '2']
+            prop2: ['1', '2']
     >>> success = atm_query_impl(tree,['prop1'], grep=True)
-        root::prop1: one
+        prop1: one
         sub::prop1: two
     """
 
     if not parent_map:
         parent_map = create_parent_map(xml_root)
     if listall:
-        print_var(xml_root,parent_map,'ANY',full,dtype,value,valid_values,"node-name","  ")
+        print_var(xml_root,parent_map,'ANY',full,dtype,value,valid_values,"node-name","    ")
 
     elif grep:
         for regex in variables:
@@ -610,7 +659,7 @@ def atm_query_impl(xml_root,variables,listall=False,full=False,value=False,
                 if len(xml_root)>0:
                     parents = get_parents(xml_root,parent_map)
                     print (f"{'::'.join([p.tag for p in parents]) + '::' + xml_root.tag}:")
-                print_var(xml_root,parent_map,'ANY',full,dtype,value,valid_values,"node-name","  ")
+                print_var(xml_root,parent_map,'ANY',full,dtype,value,valid_values,"node-name","    ")
             else:
                 for elem in xml_root:
                     if len(elem)>0:
@@ -624,6 +673,6 @@ def atm_query_impl(xml_root,variables,listall=False,full=False,value=False,
     else:
         for var in variables:
             pmap = {} if var=='ANY' else parent_map
-            print_var(xml_root,pmap,var,full,dtype,value,valid_values,"parent-scope","  ")
+            print_var(xml_root,pmap,var,full,dtype,value,valid_values,"parent-scope","    ")
 
     return True
diff --git a/components/eamxx/scripts/cime-nml-tests b/components/eamxx/scripts/cime-nml-tests
@@ -119,6 +119,7 @@ class TestBuildnml(unittest.TestCase):
     def setUp(self):
     ###########################################################################
         self._dirs_to_cleanup = []
+        self._common_case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
 
     ###########################################################################
     def tearDown(self):
@@ -174,7 +175,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that atmchanges are not lost when eamxx setup is called
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("atm_log_level", "trace"), ("output_to_screen", "true")], case)
 
@@ -192,7 +193,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that the append attribute for array-type params in namelist defaults works as expected
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         # Add testOnly proc
         self._chg_atmconfig([("mac_aero_mic::atm_procs_list", "shoc,cldFraction,spa,p3,testOnly")], case)
@@ -213,7 +214,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that var+=value syntax behaves as expected
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         # Append to an existing entry
         name = 'output_yaml_files'
@@ -230,7 +231,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that atmchanges are lost when resetting
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("atm_log_level", "trace")], case, reset=True)
 
@@ -240,7 +241,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that atmchanges are lost if SCREAM_HACK_XML=TRUE
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         run_cmd_assert_result(self, "./xmlchange SCREAM_HACK_XML=TRUE", from_dir=case)
 
@@ -267,7 +268,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that atmchange works when using 'namespace' syntax foo::bar
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("p3::enable_precondition_checks", "false")], case)
 
@@ -277,7 +278,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that atmchange works for array data
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("surf_mom_flux", "40.0,2.0")], case)
 
@@ -287,7 +288,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test that atmchange works with ANY
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("ANY::enable_precondition_checks", "false")], case)
 
@@ -297,7 +298,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test atmchange with ANY followed by an atmchange of one of them
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("ANY::enable_precondition_checks", "false"),
                              ("p3::enable_precondition_checks", "true")], case)
@@ -308,7 +309,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test atmchanges that add atm procs
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("mac_aero_mic::atm_procs_list", "shoc,cldFraction,spa,p3,testOnly")], case)
 
@@ -322,7 +323,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test atmchanges that add atm procs
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         # modifying a procs list requires known processes or "_" pre/post suffixes
         stat, out, err = run_cmd ("./atmchange mac_aero_mic::atm_procs_list=shoc,cldfraction,spa,p3,spiderman",
@@ -336,7 +337,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test atmchange does not change buffer if syntax was wrong
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         # Attempting a bad change should not alter the content of SCREAM_ATMCHANGE_BUFFER
         old = run_cmd_no_fail ("./xmlquery --value SCREAM_ATMCHANGE_BUFFER",from_dir=case)
@@ -353,7 +354,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test atmchange errors out with invalid param names
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         stat, out, err = run_cmd ("./atmchange p3::non_existent=3",from_dir=case)
         expect (stat!=0,"Command './atmchange p3::non_existent=3' should have failed")
@@ -364,7 +365,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test atmchanges that add atm proc groups
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         out = run_cmd_no_fail ("./atmchange mac_aero_mic::atm_procs_list=shoc,_my_group_",from_dir=case)
 
@@ -380,7 +381,7 @@ class TestBuildnml(unittest.TestCase):
         """
         Test atmchanges that remove atm procs
         """
-        case = self._create_test("SMS.ne30_ne30.F2010-SCREAMv1 --no-build")
+        case = self._common_case
 
         self._chg_atmconfig([("mac_aero_mic::atm_procs_list", "shoc,cldFraction,spa")], case)
 
