From f1346cfb18d2b9ab1a30f337dc5bb6d264b2acbc Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 23 Feb 2018 08:44:41 -0700 Subject: [PATCH 1/5] pass the comment from config_pes to env_mach_pes --- config/xml_schemas/env_mach_pes.xsd | 2 ++ scripts/lib/CIME/XML/env_mach_pes.py | 7 +++++++ scripts/lib/CIME/XML/generic_xml.py | 11 +++++++++-- scripts/lib/CIME/XML/pes.py | 28 +++++++++++++++++++++------- scripts/lib/CIME/case.py | 3 ++- 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/config/xml_schemas/env_mach_pes.xsd b/config/xml_schemas/env_mach_pes.xsd index df6dba2cbbe..7313797c92e 100644 --- a/config/xml_schemas/env_mach_pes.xsd +++ b/config/xml_schemas/env_mach_pes.xsd @@ -6,6 +6,7 @@ + @@ -16,6 +17,7 @@ + diff --git a/scripts/lib/CIME/XML/env_mach_pes.py b/scripts/lib/CIME/XML/env_mach_pes.py index 60e8d8f78ff..988e08988be 100644 --- a/scripts/lib/CIME/XML/env_mach_pes.py +++ b/scripts/lib/CIME/XML/env_mach_pes.py @@ -3,6 +3,7 @@ """ from CIME.XML.standard_module_setup import * from CIME.XML.env_base import EnvBase +from CIME.XML.generic_xml import GenericXML import math logger = logging.getLogger(__name__) @@ -17,6 +18,12 @@ def __init__(self, case_root=None, infile="env_mach_pes.xml", components=None): schema = os.path.join(get_cime_root(), "config", "xml_schemas", "env_mach_pes.xsd") EnvBase.__init__(self, case_root, infile, schema=schema) + def add_comment(self, comment): + if comment is not None: + tmp = GenericXML(root_name_override="comment",infile="stupid.file",read_only=False) + tmp.set_text(tmp.root, comment) + self.add_child(tmp.root, position=1) + def get_value(self, vid, attribute=None, resolved=True, subgroup=None, max_mpitasks_per_node=None): # pylint: disable=arguments-differ # Special variable NINST_MAX is used to determine the number of # drivers in multi-driver mode. diff --git a/scripts/lib/CIME/XML/generic_xml.py b/scripts/lib/CIME/XML/generic_xml.py index 128da821dca..513d25ae0af 100644 --- a/scripts/lib/CIME/XML/generic_xml.py +++ b/scripts/lib/CIME/XML/generic_xml.py @@ -170,13 +170,16 @@ def name(self, node): def text(self, node): return node.xml_element.text - def add_child(self, node, root=None): + def add_child(self, node, root=None, position=0): """ Add element node to self at root """ expect(not self.locked and not self.read_only, "locked") root = root if root is not None else self.root - root.xml_element.append(node.xml_element) + if position > 0: + root.xml_element.insert(position, node.xml_element) + else: + root.xml_element.append(node.xml_element) def copy(self, node): return deepcopy(node) @@ -199,6 +202,10 @@ def make_child(self, name, attributes=None, root=None, text=None): return node + def make_node(self, name): + return _Element(ET.Element("name")) + + def get_children(self, name=None, attributes=None, root=None): """ This is the critical function, its interface and performance are crucial. diff --git a/scripts/lib/CIME/XML/pes.py b/scripts/lib/CIME/XML/pes.py index 52d53d6a387..4e6d8522e0d 100644 --- a/scripts/lib/CIME/XML/pes.py +++ b/scripts/lib/CIME/XML/pes.py @@ -28,11 +28,14 @@ def find_pes_layout(self, grid, compset, machine, pesize_opts='M', mpilib=None): oother_settings = {} other_settings = {} o_grid_nodes = [] + comments = None # Get any override nodes overrides = self.get_optional_child("overrides") + ocomments = None if overrides is not None: o_grid_nodes = self.get_children("grid", root = overrides) - opes_ntasks, opes_nthrds, opes_rootpe, opes_pstrid, oother_settings = self._find_matches(o_grid_nodes, grid, compset, machine, pesize_opts, True) + opes_ntasks, opes_nthrds, opes_rootpe, opes_pstrid, oother_settings, ocomments = self._find_matches(o_grid_nodes, grid, compset, machine, pesize_opts, True) + # Get all the nodes grid_nodes = self.get_children("grid") if o_grid_nodes: @@ -42,12 +45,15 @@ def find_pes_layout(self, grid, compset, machine, pesize_opts='M', mpilib=None): grid_nodes = list(gn_set) - pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other_settings = self._find_matches(grid_nodes, grid, compset, machine, pesize_opts, False) + pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other_settings, comments = self._find_matches(grid_nodes, grid, compset, machine, pesize_opts, False) pes_ntasks.update(opes_ntasks) pes_nthrds.update(opes_nthrds) pes_rootpe.update(opes_rootpe) pes_pstrid.update(opes_pstrid) other_settings.update(oother_settings) + if ocomments is not None: + comments = ocomments + if mpilib == "mpi-serial": for i in iter(pes_ntasks): @@ -64,7 +70,10 @@ def find_pes_layout(self, grid, compset, machine, pesize_opts='M', mpilib=None): logger.info("Pes setting: rootpe is {} ".format(pes_rootpe)) logger.info("Pes setting: pstrid is {} ".format(pes_pstrid)) logger.info("Pes other settings: {}".format(other_settings)) - return pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other_settings + if comments is not None: + logger.info("Pes comments: {}".format(comments)) + + return pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other_settings, comments def _find_matches(self, grid_nodes, grid, compset, machine, pesize_opts, override=False): grid_choice = None @@ -74,6 +83,7 @@ def _find_matches(self, grid_nodes, grid, compset, machine, pesize_opts, overrid max_points = -1 pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other_settings = {}, {}, {}, {}, {} pe_select = None + comment = None for grid_node in grid_nodes: grid_match = self.get(grid_node, "name") if grid_match == "any" or re.search(grid_match,grid): @@ -96,7 +106,9 @@ def _find_matches(self, grid_nodes, grid, compset, machine, pesize_opts, overrid for node in self.get_children(root=pes_node): vid = self.name(node) logger.info("vid is {}".format(vid)) - if "ntasks" in vid: + if "comment" in vid: + comment = self.text(node) + elif "ntasks" in vid: for child in self.get_children(root=node): pes_ntasks[self.name(child).upper()] = int(self.text(child)) elif "nthrds" in vid: @@ -131,7 +143,9 @@ def _find_matches(self, grid_nodes, grid, compset, machine, pesize_opts, overrid for node in self.get_children(root=pe_select): vid = self.name(node) logger.debug("vid is {}".format(vid)) - if "ntasks" in vid: + if "comment" in vid: + comment = self.text(node) + elif "ntasks" in vid: for child in self.get_children(root=node): pes_ntasks[self.name(child).upper()] = int(self.text(child)) elif "nthrds" in vid: @@ -144,7 +158,7 @@ def _find_matches(self, grid_nodes, grid, compset, machine, pesize_opts, overrid for child in self.get_children(root=node): pes_pstrid[self.name(child).upper()] = int(self.text(child)) # if the value is already upper case its something else we are trying to set - elif vid == self.name(node) and vid != "comment": + elif vid == self.name(node): other_settings[vid] = self.text(node) if grid_choice != 'any' or logger.isEnabledFor(logging.DEBUG): logger.info("Pes setting: grid match is {} ".format(grid_choice )) @@ -155,4 +169,4 @@ def _find_matches(self, grid_nodes, grid, compset, machine, pesize_opts, overrid if pesize_choice != 'any' or logger.isEnabledFor(logging.DEBUG): logger.info("Pes setting: pesize match is {} ".format(pesize_choice)) - return pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other_settings + return pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other_settings, comment diff --git a/scripts/lib/CIME/case.py b/scripts/lib/CIME/case.py index 9f47e181c37..396f6b2ff3a 100644 --- a/scripts/lib/CIME/case.py +++ b/scripts/lib/CIME/case.py @@ -687,7 +687,7 @@ def _setup_mach_pes(self, pecount, multi_driver, ninst, machine_name, mpilib): force_tasks = int(match2.group(1)) pes_nthrds = pesobj.find_pes_layout(self._gridname, self._compsetname, machine_name, mpilib=mpilib)[1] else: - pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other = pesobj.find_pes_layout(self._gridname, self._compsetname, + pes_ntasks, pes_nthrds, pes_rootpe, pes_pstrid, other, comment = pesobj.find_pes_layout(self._gridname, self._compsetname, machine_name, pesize_opts=pecount, mpilib=mpilib) if match1 or match2: @@ -705,6 +705,7 @@ def _setup_mach_pes(self, pecount, multi_driver, ninst, machine_name, mpilib): pes_rootpe[string_] = 0 mach_pes_obj = self.get_env("mach_pes") + mach_pes_obj.add_comment(comment) if other is not None: for key, value in other.items(): From 39d868aba0da861cf4defd04a0bfedf5cea4c07b Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 23 Feb 2018 12:01:29 -0700 Subject: [PATCH 2/5] all tests passing --- scripts/lib/CIME/XML/env_mach_pes.py | 2 +- scripts/lib/CIME/XML/generic_xml.py | 15 ++++----------- scripts/lib/CIME/case.py | 2 +- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/scripts/lib/CIME/XML/env_mach_pes.py b/scripts/lib/CIME/XML/env_mach_pes.py index 988e08988be..d00df240b18 100644 --- a/scripts/lib/CIME/XML/env_mach_pes.py +++ b/scripts/lib/CIME/XML/env_mach_pes.py @@ -20,7 +20,7 @@ def __init__(self, case_root=None, infile="env_mach_pes.xml", components=None): def add_comment(self, comment): if comment is not None: - tmp = GenericXML(root_name_override="comment",infile="stupid.file",read_only=False) + tmp = GenericXML(root_name_override="comment", infile="fake", read_only=False) tmp.set_text(tmp.root, comment) self.add_child(tmp.root, position=1) diff --git a/scripts/lib/CIME/XML/generic_xml.py b/scripts/lib/CIME/XML/generic_xml.py index 513d25ae0af..94b3e24ce32 100644 --- a/scripts/lib/CIME/XML/generic_xml.py +++ b/scripts/lib/CIME/XML/generic_xml.py @@ -44,10 +44,7 @@ def __init__(self, infile=None, schema=None, root_name_override=None, root_attri self.root = None self.locked = False self.read_only = read_only - - if infile == None: - # if file is not defined just return - self.filename = None + if infile is None: return if os.path.isfile(infile) and os.access(infile, os.R_OK): @@ -57,13 +54,13 @@ def __init__(self, infile=None, schema=None, root_name_override=None, root_attri else: # if file does not exist create a root xml element # and set it's id to file - expect(not read_only, "Makes no sense to have empty read-only file") - + expect(not self.read_only, "Makes no sense to have empty read-only file") logger.debug("File {} does not exists.".format(infile)) expect("$" not in infile,"File path not fully resolved {}".format(infile)) - self.filename = infile + root = _Element(ET.Element("xml")) + if root_name_override: self.root = self.make_child(root_name_override, root=root, attributes=root_attrib_override) else: @@ -202,10 +199,6 @@ def make_child(self, name, attributes=None, root=None, text=None): return node - def make_node(self, name): - return _Element(ET.Element("name")) - - def get_children(self, name=None, attributes=None, root=None): """ This is the critical function, its interface and performance are crucial. diff --git a/scripts/lib/CIME/case.py b/scripts/lib/CIME/case.py index 396f6b2ff3a..b151ad125b2 100644 --- a/scripts/lib/CIME/case.py +++ b/scripts/lib/CIME/case.py @@ -671,7 +671,7 @@ def _setup_mach_pes(self, pecount, multi_driver, ninst, machine_name, mpilib): pes_rootpe = {} pes_pstrid = {} other = {} - + comment = None force_tasks = None force_thrds = None From cb3d1022384ed5e7a1243780ff5869ba2b279d3d Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 23 Feb 2018 12:47:47 -0700 Subject: [PATCH 3/5] cleanup; --- scripts/lib/CIME/XML/generic_xml.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/lib/CIME/XML/generic_xml.py b/scripts/lib/CIME/XML/generic_xml.py index 94b3e24ce32..6115ca721ed 100644 --- a/scripts/lib/CIME/XML/generic_xml.py +++ b/scripts/lib/CIME/XML/generic_xml.py @@ -44,12 +44,12 @@ def __init__(self, infile=None, schema=None, root_name_override=None, root_attri self.root = None self.locked = False self.read_only = read_only + self.filename = infile if infile is None: return if os.path.isfile(infile) and os.access(infile, os.R_OK): # If file is defined and exists, read it - self.filename = infile self.read(infile, schema) else: # if file does not exist create a root xml element @@ -57,7 +57,6 @@ def __init__(self, infile=None, schema=None, root_name_override=None, root_attri expect(not self.read_only, "Makes no sense to have empty read-only file") logger.debug("File {} does not exists.".format(infile)) expect("$" not in infile,"File path not fully resolved {}".format(infile)) - self.filename = infile root = _Element(ET.Element("xml")) From 31163b71ec7970d22e2c72d81a4c028d2c50caa5 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 23 Feb 2018 14:15:31 -0700 Subject: [PATCH 4/5] response to review --- scripts/lib/CIME/XML/env_mach_pes.py | 8 +++++--- scripts/lib/CIME/XML/generic_xml.py | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/lib/CIME/XML/env_mach_pes.py b/scripts/lib/CIME/XML/env_mach_pes.py index d00df240b18..700a406332a 100644 --- a/scripts/lib/CIME/XML/env_mach_pes.py +++ b/scripts/lib/CIME/XML/env_mach_pes.py @@ -20,9 +20,11 @@ def __init__(self, case_root=None, infile="env_mach_pes.xml", components=None): def add_comment(self, comment): if comment is not None: - tmp = GenericXML(root_name_override="comment", infile="fake", read_only=False) - tmp.set_text(tmp.root, comment) - self.add_child(tmp.root, position=1) + node = self.make_child("comment", text=comment) + # make_child adds to the end of the file but we want it to follow the header + # so we need to remove it and add it in the correct position + self.remove_child(node) + self.add_child(node, position=1) def get_value(self, vid, attribute=None, resolved=True, subgroup=None, max_mpitasks_per_node=None): # pylint: disable=arguments-differ # Special variable NINST_MAX is used to determine the number of diff --git a/scripts/lib/CIME/XML/generic_xml.py b/scripts/lib/CIME/XML/generic_xml.py index 6115ca721ed..ac763f1e8dc 100644 --- a/scripts/lib/CIME/XML/generic_xml.py +++ b/scripts/lib/CIME/XML/generic_xml.py @@ -166,13 +166,13 @@ def name(self, node): def text(self, node): return node.xml_element.text - def add_child(self, node, root=None, position=0): + def add_child(self, node, root=None, position=None): """ Add element node to self at root """ expect(not self.locked and not self.read_only, "locked") root = root if root is not None else self.root - if position > 0: + if position is not None: root.xml_element.insert(position, node.xml_element) else: root.xml_element.append(node.xml_element) From 1ac2fb69d981e6f275df4538d05ad477075c0fb5 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Fri, 23 Feb 2018 14:18:02 -0700 Subject: [PATCH 5/5] fix pylint issue --- scripts/lib/CIME/XML/env_mach_pes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/lib/CIME/XML/env_mach_pes.py b/scripts/lib/CIME/XML/env_mach_pes.py index 700a406332a..6ee18623a12 100644 --- a/scripts/lib/CIME/XML/env_mach_pes.py +++ b/scripts/lib/CIME/XML/env_mach_pes.py @@ -3,7 +3,6 @@ """ from CIME.XML.standard_module_setup import * from CIME.XML.env_base import EnvBase -from CIME.XML.generic_xml import GenericXML import math logger = logging.getLogger(__name__)