Merge pull request #1965 from jedwards4b/iscompvar_cleanup

Iscompvar cleanup

This is a cleanup of the compvar implementation. There was a confusion between component attributes which represented components (cam, datm, etc) vs those which represented component classes (ATM, LND, etc). I've renamed the attributes associated with component classes to be compclass instead of component. This change also allowed me to get rid of the explicit list of compvars in env_mach_pes.py and env_run.py

Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes:

User interface changes?:

Update gh-pages html (Y/N)?:

Code review:

config/xml_schemas/env_mach_pes.xsd

@@ -57,7 +57,7 @@
     <xs:complexType>
       <xs:simpleContent>
         <xs:extension base="xs:NMTOKEN">
-          <xs:attribute name="component" use="required" type="xs:NCName"/>
+          <xs:attribute name="compclass" use="required" type="xs:NCName"/>
         </xs:extension>
       </xs:simpleContent>
     </xs:complexType>

scripts/Tools/xmlquery

@@ -244,9 +244,9 @@ def xmlquery_sub(case, variables, subgroup=None, fileonly=False,
                     value = []
                     for comp in comp_classes:
                         try:
-                            nextval = get_value_as_string(case,var, attribute={"component" : comp}, resolved=resolved, subgroup=group)
+                            nextval = get_value_as_string(case,var, attribute={"compclass" : comp}, resolved=resolved, subgroup=group)
                         except:
-                            nextval = get_value_as_string(case,var, attribute={"component" : comp}, resolved=False, subgroup=group)
+                            nextval = get_value_as_string(case,var, attribute={"compclass" : comp}, resolved=False, subgroup=group)
 
                         if nextval is not None:
                             value.append(comp + ":" + "%s"%nextval)

scripts/lib/CIME/XML/env_base.py

@@ -30,32 +30,35 @@ def set_components(self, components):
             self._components = components
 
     def check_if_comp_var(self, vid, attribute=None):
-        # pylint: disable=no-member
-        if not hasattr(self, "_component_value_list") or\
-                (self.get_nodes("entry", {"id" : vid}) and \
-                     not vid in self._component_value_list):
-            return vid, None, False
-
+        nodes = self.get_nodes("entry", {"id" : vid})
+        node = None
         comp = None
-        if vid in self._component_value_list:
-            if attribute is not None:
-                if "component" in attribute:
-                    comp = attribute["component"]
-
-            return vid, comp, True
-
-        new_vid = None
-        for comp in self._components:
-            if "_"+comp in vid:
-                new_vid = vid.replace('_'+comp, '', 1)
-            elif comp+"_" in vid:
-                new_vid = vid.replace(comp+'_', '', 1)
-
-            if new_vid is not None:
-                break
+        if len(nodes):
+            node = nodes[0]
+        if node:
+            valnodes = node.findall(".//value[@compclass]")
+            if len(valnodes) == 0:
+                logger.debug("vid {} is not a compvar".format(vid))
+                return vid, None, False
+            else:
+                logger.debug("vid {} is a compvar".format(vid))
+                if attribute is not None:
+                    comp = attribute["compclass"]
+                return vid, comp, True
+        else:
+            if hasattr(self, "_components"):
+                new_vid = None
+                for comp in self._components:
+                    if "_"+comp in vid:
+                        new_vid = vid.replace('_'+comp, '', 1)
+                    elif comp+"_" in vid:
+                        new_vid = vid.replace(comp+'_', '', 1)
 
-        if new_vid is not None and new_vid in self._component_value_list:
-            return new_vid, comp, True
+                    if new_vid is not None:
+                        break
+                if new_vid is not None:
+                    logger.debug("vid {} is a compvar with comp {}".format(vid, comp))
+                    return new_vid, comp, True
 
         return vid, None, False
 
@@ -76,9 +79,9 @@ def get_value(self, vid, attribute=None, resolved=True, subgroup=None):
                     logger.debug("Not enough info to get value for {}".format(vid))
                     return value
             if attribute is None:
-                attribute = {"component" : comp}
+                attribute = {"compclass" : comp}
             else:
-                attribute["component"] = comp
+                attribute["compclass"] = comp
             node = self.get_optional_node("entry", {"id":vid})
             if node is not None:
                 type_str = self._get_type_info(node)
@@ -105,19 +108,19 @@ def set_value(self, vid, value, subgroup=None, ignore_type=False):
             if iscompvar and comp is None:
                 # pylint: disable=no-member
                 for comp in self._components:
-                    val = self._set_value(node, value, vid, subgroup, ignore_type, component=comp)
+                    val = self._set_value(node, value, vid, subgroup, ignore_type, compclass=comp)
             else:
-                val = self._set_value(node, value, vid, subgroup, ignore_type, component=comp)
+                val = self._set_value(node, value, vid, subgroup, ignore_type, compclass=comp)
         return val
 
     # pylint: disable=arguments-differ
-    def _set_value(self, node, value, vid=None, subgroup=None, ignore_type=False, component=None):
+    def _set_value(self, node, value, vid=None, subgroup=None, ignore_type=False, compclass=None):
         if vid is None:
             vid = node.get("id")
         vid, _, iscompvar = self.check_if_comp_var(vid, None)
 
         if iscompvar:
-            attribute = {"component":component}
+            attribute = {"compclass":compclass}
             str_value = self.get_valid_value_string(node, value, vid, ignore_type)
             val = self.set_element_text("value", str_value, attribute, root=node)
         else:
@@ -140,8 +143,18 @@ def cleanupnode(self, node):
         if dnode is not None:
             node.remove(dnode)
         vnode = node.find(".//values")
-        vid = node.get("id")
-        _, _, iscompvar = self.check_if_comp_var(vid)
-        if vnode is not None and not iscompvar:
-            node.remove(vnode)
+        if vnode is not None:
+            componentatt = vnode.findall(".//value[@component=\"ATM\"]")
+            # backward compatibility (compclasses and component were mixed
+            # now we seperated into component and compclass)
+            if len(componentatt) > 0:
+                for ccnode in vnode.findall(".//value[@component]"):
+                    val = ccnode.attrib.get("component")
+                    ccnode.attrib.pop("component")
+                    ccnode.set("compclass",val)
+            compclassatt = vnode.findall(".//value[@compclass]")
+
+            if len(compclassatt) == 0:
+                node.remove(vnode)
+
         return node

scripts/lib/CIME/XML/env_mach_pes.py

@@ -14,8 +14,6 @@ def __init__(self, case_root=None, infile="env_mach_pes.xml", components=None):
         initialize an object interface to file env_mach_pes.xml in the case directory
         """
         self._components = components
-        self._component_value_list = ["NTASKS", "NTHRDS", "NINST",
-                                      "ROOTPE", "PSTRID", "NINST_LAYOUT", "NTASKS_PER_INST"]
         schema = os.path.join(get_cime_root(), "config", "xml_schemas", "env_mach_pes.xsd")
         EnvBase.__init__(self, case_root, infile, schema=schema)
 
@@ -61,7 +59,7 @@ def get_max_thread_count(self, comp_classes):
         ''' Find the maximum number of openmp threads for any component in the case '''
         max_threads = 1
         for comp in comp_classes:
-            threads = self.get_value("NTHRDS",attribute={"component":comp})
+            threads = self.get_value("NTHRDS",attribute={"compclass":comp})
             expect(threads is not None, "Error no thread count found for component class {}".format(comp))
             if threads > max_threads:
                 max_threads = threads
@@ -86,11 +84,11 @@ def get_total_tasks(self, comp_classes):
         total_tasks = 0
         maxinst = 1
         for comp in comp_classes:
-            ntasks = self.get_value("NTASKS", attribute={"component":comp})
-            rootpe = self.get_value("ROOTPE", attribute={"component":comp})
-            pstrid = self.get_value("PSTRID", attribute={"component":comp})
+            ntasks = self.get_value("NTASKS", attribute={"compclass":comp})
+            rootpe = self.get_value("ROOTPE", attribute={"compclass":comp})
+            pstrid = self.get_value("PSTRID", attribute={"compclass":comp})
             if comp != "CPL":
-                ninst = self.get_value("NINST", attribute={"component":comp})
+                ninst = self.get_value("NINST", attribute={"compclass":comp})
                 maxinst = max(maxinst, ninst)
             tt = rootpe + (ntasks - 1) * pstrid + 1
             total_tasks = max(tt, total_tasks)
@@ -99,7 +97,7 @@ def get_total_tasks(self, comp_classes):
         return total_tasks
 
     def get_tasks_per_node(self, total_tasks, max_thread_count):
-        expect(total_tasks > 0,"totaltasks > 0 expected totaltasks = {}".format(total_tasks))
+        expect(total_tasks > 0,"totaltasks > 0 expected, totaltasks = {}".format(total_tasks))
         tasks_per_node = min(self.get_value("MAX_TASKS_PER_NODE")/ max_thread_count,
                              self.get_value("MAX_MPITASKS_PER_NODE"), total_tasks)
         return tasks_per_node if tasks_per_node > 0 else 1

scripts/lib/CIME/XML/env_run.py

@@ -14,8 +14,6 @@ def __init__(self, case_root=None, infile="env_run.xml", components=None):
         initialize an object interface to file env_run.xml in the case directory
         """
         self._components = components
-        self._component_value_list = ["PIO_TYPENAME", "PIO_STRIDE", "PIO_REARRANGER",
-                                      "PIO_NUMTASKS", "PIO_ROOT", "PIO_NETCDF_FORMAT"]
         schema = os.path.join(get_cime_root(), "config", "xml_schemas", "env_entry_id.xsd")
 
         EnvBase.__init__(self, case_root, infile, schema=schema)

scripts/lib/CIME/case.py

@@ -129,7 +129,6 @@ def initialize_derived_attributes(self):
         env_mach_spec = self.get_env('mach_specific')
         comp_classes  = self.get_values("COMP_CLASSES")
         MAX_MPITASKS_PER_NODE  = self.get_value("MAX_MPITASKS_PER_NODE")
-
         self.total_tasks = env_mach_pes.get_total_tasks(comp_classes)
         self.thread_count = env_mach_pes.get_max_thread_count(comp_classes)
         self.tasks_per_node = env_mach_pes.get_tasks_per_node(self.total_tasks, self.thread_count)

scripts/lib/CIME/utils.py

@@ -1233,6 +1233,7 @@ def find_system_test(testname, case):
                         if system_test_dir not in sys.path:
                             sys.path.append(system_test_dir)
                         system_test_path = "{}.{}".format(testname.lower(), testname)
+        expect(len(fdir) > 0, "Test {} not found, aborting".format(testname))
         expect(len(fdir) == 1, "Test {} found in multiple locations {}, aborting".format(testname, fdir))
     expect(system_test_path is not None, "No test {} found".format(testname))