Merge pull request #1549 from ACME-Climate/azamat/mira/add-smp-present

Define threading env-vars only for threaded builds (PR #1549)

* Adds SMP_PRESENT case-var, which is true if (BUILD_THREADED or NTHRDS_* >1)
* Enable/disable env_mach_specific XML variables based on attributes set in the case variables

[BFB]

config/acme/machines/config_machines.xml

@@ -1126,16 +1126,11 @@
          <mpirun mpilib="default">
            <executable>/usr/bin/runjob</executable>
              <arguments>
-               <arg name="label"> --label short</arg>
-               <!-- Ranks per node!! -->
-               <arg name="tasks_per_node"> --ranks-per-node $PES_PER_NODE</arg>
-               <!-- Total MPI Tasks -->
-               <arg name="num_tasks"> --np $TOTALPES</arg>
-               <arg name="locargs"> --block $COBALT_PARTNAME $LOCARGS</arg>
-               <arg name="bg_threadlayout"> --envs BG_THREADLAYOUT=1</arg>
-	       <arg name="xl_bg_spreadlayout"> --envs XL_BG_SPREADLAYOUT=YES</arg>
-               <arg name="omp_stacksize"> --envs OMP_STACKSIZE=64M</arg>
-               <arg name="thread_count"> --envs OMP_NUM_THREADS=$ENV{OMP_NUM_THREADS}</arg>
+               <arg name="label">--label short</arg>
+               <arg name="tasks_per_node">--ranks-per-node $PES_PER_NODE</arg>
+               <arg name="num_tasks">--np $TOTALPES</arg>
+               <arg name="locargs">--block $COBALT_PARTNAME $LOCARGS</arg>
+               <arg name="bgq_smp_vars">$ENV{BGQ_SMP_VARS}</arg>
              </arguments>
          </mpirun>
     <module_system type="soft">
@@ -1152,8 +1147,10 @@
     </module_system>
     <environment_variables>
       <env name="MPI_TYPE_MAX">10000</env>
-      <env name="OMP_DYNAMIC">FALSE</env>
-      <env name="OMP_STACKSIZE">64M</env>
+      <env name="BGQ_SMP_VARS"></env>
+    </environment_variables>
+    <environment_variables SMP_PRESENT="TRUE">
+      <env name="BGQ_SMP_VARS">--envs BG_THREADLAYOUT=1 XL_BG_SPREADLAYOUT=YES OMP_DYNAMIC=FALSE OMP_STACKSIZE=64M OMP_NUM_THREADS=$ENV{OMP_NUM_THREADS}</env>
     </environment_variables>
 </machine>
 
@@ -1289,16 +1286,11 @@
          <mpirun mpilib="default">
            <executable>/usr/bin/runjob</executable>
              <arguments>
-               <arg name="label"> --label short</arg>
-               <!-- Ranks per node!! -->
-               <arg name="tasks_per_node"> --ranks-per-node $PES_PER_NODE</arg>
-               <!-- Total MPI Tasks -->
-               <arg name="num_tasks"> --np $TOTALPES</arg>
-               <arg name="locargs"> --block $COBALT_PARTNAME $LOCARGS</arg>
-               <arg name="bg_threadlayout"> --envs BG_THREADLAYOUT=1</arg>
-	       <arg name="xl_bg_spreadlayout"> --envs XL_BG_SPREADLAYOUT=YES</arg>
-               <arg name="omp_stacksize"> --envs OMP_STACKSIZE=64M</arg>
-               <arg name="thread_count"> --envs OMP_NUM_THREADS=$ENV{OMP_NUM_THREADS}</arg>
+               <arg name="label">--label short</arg>
+               <arg name="tasks_per_node">--ranks-per-node $PES_PER_NODE</arg>
+               <arg name="num_tasks">--np $TOTALPES</arg>
+               <arg name="locargs">--block $COBALT_PARTNAME $LOCARGS</arg>
+               <arg name="bgq_smp_vars">$ENV{BGQ_SMP_VARS}</arg>
              </arguments>
          </mpirun>
     <module_system type="soft">
@@ -1315,8 +1307,10 @@
     </module_system>
     <environment_variables>
       <env name="MPI_TYPE_MAX">10000</env>
-      <env name="OMP_DYNAMIC">FALSE</env>
-      <env name="OMP_STACKSIZE">64M</env>
+      <env name="BGQ_SMP_VARS"></env>
+    </environment_variables>
+    <environment_variables SMP_PRESENT="TRUE">
+      <env name="BGQ_SMP_VARS">--envs BG_THREADLAYOUT=1 XL_BG_SPREADLAYOUT=YES OMP_DYNAMIC=FALSE OMP_STACKSIZE=64M OMP_NUM_THREADS=$ENV{OMP_NUM_THREADS}</env>
     </environment_variables>
 </machine>
 

config/config_tests.xml

@@ -245,6 +245,7 @@ NODEFAIL          Tests restart upon detected node failure. Generates fake failu
     <STOP_OPTION>ndays</STOP_OPTION>
     <STOP_N>11</STOP_N>
     <DOUT_S>FALSE</DOUT_S>
+    <BUILD_THREADED>TRUE</BUILD_THREADED>
   </test>
 
   <test NAME="ERS">

scripts/Tools/preview_run

@@ -58,6 +58,7 @@ def _main_func(description):
 
     with Case(caseroot, read_only=False) as case:
         print "BATCH SUBMIT:"
+        case.load_env()
         job = "case.test" if case.get_value("TEST") else "case.run"
         job_id_to_cmd = case.submit_jobs(dry_run=True, job=job)
         for job_id, cmd in job_id_to_cmd:

scripts/fortran_unit_testing/run_tests.py

@@ -7,7 +7,7 @@
 sys.path.append(os.path.join(_CIMEROOT, "scripts", "fortran_unit_testing", "python"))
 
 from standard_script_setup import *
-from CIME.BuildTools.configure import configure
+from CIME.BuildTools.configure import configure, FakeCase
 from CIME.utils import run_cmd_no_fail, stringify_bool, expect
 from CIME.XML.machines import Machines
 from CIME.XML.compilers import Compilers
@@ -318,7 +318,8 @@ def _main():
               unit_testing=True)
     machspecific = EnvMachSpecific(build_dir, unit_testing=True)
 
-    machspecific.load_env(compiler, debug, mpilib)
+    fake_case = FakeCase(compiler, mpilib, debug)
+    machspecific.load_env(fake_case)
     os.environ["OS"] = os_
     os.environ["COMPILER"] = compiler
     os.environ["DEBUG"] = stringify_bool(debug)

scripts/lib/CIME/BuildTools/configure.py

@@ -66,6 +66,14 @@ def _copy_depends_files(machine_name, machines_dir, output_dir, compiler):
             if os.path.isfile(dfile) and not os.path.isfile(outputdfile):
                 shutil.copyfile(dfile, outputdfile)
 
+class FakeCase(object):
+
+    def __init__(self, compiler, mpilib, debug):
+        self._vals = {"COMPILER":compiler, "MPILIB":mpilib, "DEBUG":debug}
+
+    def get_value(self, attrib):
+        expect(attrib in self._vals, "FakeCase does not support getting value of '%s'" % attrib)
+        return self._vals[attrib]
 
 def _generate_env_mach_specific(output_dir, machobj, compiler, mpilib, debug,
                                 sysos, unit_testing):
@@ -79,8 +87,9 @@ def _generate_env_mach_specific(output_dir, machobj, compiler, mpilib, debug,
     ems_file = EnvMachSpecific(output_dir, unit_testing=unit_testing)
     ems_file.populate(machobj)
     ems_file.write()
+    fake_case = FakeCase(compiler, mpilib, debug)
     for shell in ('sh', 'csh'):
-        ems_file.make_env_mach_specific_file(compiler, debug, mpilib, shell)
+        ems_file.make_env_mach_specific_file(shell, fake_case)
         shell_path = os.path.join(output_dir, ".env_mach_specific." + shell)
         with open(shell_path, 'a') as shell_file:
             if shell == 'sh':

scripts/lib/CIME/SystemTests/erp.py

@@ -31,7 +31,6 @@ def build_phase(self, sharedlib_only=False, model_only=False):
         and tasks. This test will fail for components (e.g. pop) that do not reproduce exactly
         with different numbers of mpi tasks.
         """
-        self._case.set_value("BUILD_THREADED",True)
         if sharedlib_only:
             return self.build_indv(sharedlib_only=sharedlib_only, model_only=model_only)
 
@@ -52,6 +51,8 @@ def build_phase(self, sharedlib_only=False, model_only=False):
         if is_locked(envbuild1):
             restore(envbuild1, newname="env_build.xml")
 
+        self._case.read_xml()
+
         # Build two executables, one using the original tasks and threads (ERP1) and
         # one using the modified tasks and threads (ERP2)
         # The reason we currently need two executables that CESM-CICE has a compile time decomposition
@@ -132,6 +133,8 @@ def run_phase(self):
                 self._case.set_value("CONTINUE_RUN", True)
                 self._case.set_value("REST_OPTION","never")
                 suffix = "rest"
+
             self.run_indv(suffix=suffix)
+            self._case.flush()
 
         self._component_compare_test("base", "rest")

scripts/lib/CIME/XML/env_mach_specific.py

@@ -53,34 +53,34 @@ def populate(self, machobj):
                 for node in nodes:
                     self.add_child(node)
 
-    def _get_modules_for_case(self, compiler, debug, mpilib):
+    def _get_modules_for_case(self, case):
         module_nodes = self.get_nodes("modules")
         modules_to_load = None
         if module_nodes is not None:
-            modules_to_load = self._compute_module_actions(module_nodes, compiler, debug, mpilib)
+            modules_to_load = self._compute_module_actions(module_nodes, case)
 
         return modules_to_load
 
-    def _get_envs_for_case(self, compiler, debug, mpilib):
+    def _get_envs_for_case(self, case):
         env_nodes = self.get_nodes("environment_variables")
 
         envs_to_set = None
         if env_nodes is not None:
-            envs_to_set = self._compute_env_actions(env_nodes, compiler, debug, mpilib)
+            envs_to_set = self._compute_env_actions(env_nodes, case)
 
         return envs_to_set
 
-    def load_env(self, compiler, debug, mpilib):
+    def load_env(self, case):
         """
         Should only be called by case.load_env
         """
         # Do the modules so we can refer to env vars set by the modules
         # in the environment_variables block
-        modules_to_load = self._get_modules_for_case(compiler, debug, mpilib)
+        modules_to_load = self._get_modules_for_case(case)
         if (modules_to_load is not None):
             self.load_modules(modules_to_load)
 
-        envs_to_set = self._get_envs_for_case(compiler, debug, mpilib)
+        envs_to_set = self._get_envs_for_case(case)
         if (envs_to_set is not None):
             self.load_envs(envs_to_set)
 
@@ -130,9 +130,9 @@ def save_all_env_info(self, filename):
             f.write(self.list_modules())
         run_cmd_no_fail("echo -e '\n' && env", arg_stdout=filename)
 
-    def make_env_mach_specific_file(self, compiler, debug, mpilib, shell):
-        modules_to_load = self._get_modules_for_case(compiler, debug, mpilib)
-        envs_to_set = self._get_envs_for_case(compiler, debug, mpilib)
+    def make_env_mach_specific_file(self, shell, case):
+        modules_to_load = self._get_modules_for_case(case)
+        envs_to_set = self._get_envs_for_case(case)
         filename = ".env_mach_specific.%s" % shell
         lines = []
         if modules_to_load is not None:
@@ -152,24 +152,25 @@ def make_env_mach_specific_file(self, compiler, debug, mpilib, shell):
 
     def load_envs(self, envs_to_set):
         for env_name, env_value in envs_to_set:
-            os.environ[env_name] = env_value
+            os.environ[env_name] = "" if env_value is None else env_value
 
     # Private API
 
-    def _compute_module_actions(self, module_nodes, compiler, debug, mpilib):
-        return self._compute_actions(module_nodes, "command", compiler, debug, mpilib)
+    def _compute_module_actions(self, module_nodes, case):
+        return self._compute_actions(module_nodes, "command", case)
 
-    def _compute_env_actions(self, env_nodes, compiler, debug, mpilib):
-        return self._compute_actions(env_nodes, "env", compiler, debug, mpilib)
+    def _compute_env_actions(self, env_nodes, case):
+        return self._compute_actions(env_nodes, "env", case)
 
-    def _compute_actions(self, nodes, child_tag, compiler, debug, mpilib):
+    def _compute_actions(self, nodes, child_tag, case):
         result = [] # list of tuples ("name", "argument")
+        compiler, mpilib = case.get_value("COMPILER"), case.get_value("MPILIB")
 
         for node in nodes:
-            if (self._match_attribs(node.attrib, compiler, debug, mpilib)):
+            if (self._match_attribs(node.attrib, case)):
                 for child in node:
                     expect(child.tag == child_tag, "Expected %s element" % child_tag)
-                    if (self._match_attribs(child.attrib, compiler, debug, mpilib)):
+                    if (self._match_attribs(child.attrib, case)):
                         val = child.text
                         if val is not None:
                             # We allow a couple special substitutions for these fields
@@ -178,36 +179,39 @@ def _compute_actions(self, nodes, child_tag, compiler, debug, mpilib):
 
                             val = self.get_resolved_value(val)
                             expect("$" not in val, "Not safe to leave unresolved items in env var value: '%s'" % val)
+
                         # intentional unindent, result is appended even if val is None
                         result.append( (child.get("name"), val) )
 
         return result
 
-    def _match_attribs(self, attribs, compiler, debug, mpilib):
-        if ("compiler" in attribs and
-            not self._match(compiler, attribs["compiler"])):
-            return False
-        elif ("mpilib" in attribs and
-            not self._match(mpilib, attribs["mpilib"])):
-            return False
-        elif ("debug" in attribs and
-            not self._match("TRUE" if debug else "FALSE", attribs["debug"].upper())):
-            return False
-        elif ("unit_testing" in attribs and
-              not self._match("TRUE" if self._unit_testing else "FALSE",
-                              attribs["unit_testing"].upper())):
-            return False
+    def _match_attribs(self, attribs, case):
+        # check for matches with case-vars
+        for attrib in attribs:
+            if attrib == "unit_testing": # special case
+                if not self._match(self._unit_testing, attribs["unit_testing"].upper()):
+                    return False
+            elif attrib == "name":
+                pass
+            else:
+                val = case.get_value(attrib.upper())
+                expect(val is not None, "Cannot match attrib '%s', case has no value for it" % attrib.upper())
+                if not self._match(val, attribs[attrib]):
+                    return False
 
         return True
 
     def _match(self, my_value, xml_value):
-        if (xml_value.startswith("!")):
+        if xml_value.startswith("!"):
             result = my_value != xml_value[1:]
+        elif isinstance(my_value, bool):
+            if my_value: result = xml_value == "TRUE"
+            else: result = xml_value == "FALSE"
         else:
             result = my_value == xml_value
-        logger.debug("(env_mach_specific) _match %s %s %s"%(my_value, xml_value, result))
-        return result
 
+        logger.debug("(env_mach_specific) _match %s %s %s" % (my_value, xml_value, result))
+        return result
 
     def _get_module_commands(self, modules_to_load, shell):
         # Note this is independent of module system type

scripts/lib/CIME/XML/generic_xml.py

@@ -4,7 +4,6 @@
 """
 from CIME.XML.standard_module_setup import *
 from distutils.spawn import find_executable
-from xml.dom import minidom
 
 import getpass
 
@@ -47,11 +46,12 @@ def read(self, infile, schema=None):
         Read and parse an xml file into the object
         """
         logger.debug("read: " + infile)
-        if self.tree:
-            self.root.append(ET.parse(infile).getroot())
-        else:
-            self.tree = ET.parse(infile)
-            self.root = self.tree.getroot()
+        with open(infile, 'r') as fd:
+            if self.tree:
+                self.root.append(ET.parse(fd).getroot())
+            else:
+                self.tree = ET.parse(fd)
+                self.root = self.tree.getroot()
 
         if schema is not None and self.get_version() > 1.0:
             self.validate_xml_file(infile, schema)
@@ -79,9 +79,8 @@ def write(self, outfile=None):
         if xmllint is not None:
             run_cmd_no_fail("%s --format --output %s -"%(xmllint,outfile), input_str=xmlstr)
         else:
-            doc = minidom.parseString(xmlstr)
             with open(outfile,'w') as xmlout:
-                doc.writexml(xmlout,addindent='  ')
+                xmlout.write(xmlstr)
 
     def get_node(self, nodename, attributes=None, root=None, xpath=None):
         """

scripts/lib/CIME/build.py

@@ -267,7 +267,6 @@ def _build_libraries(case, exeroot, sharedpath, caseroot, cimeroot, libroot, lid
                 if re.search("Current setting for", line):
                     logger.warn(line)
 
-
     # clm not a shared lib for ACME
     if get_model() != "acme":
         comp_lnd = case.get_value("COMP_LND")
@@ -520,7 +519,6 @@ def _case_build_impl(caseroot, case, sharedlib_only, model_only):
         logs.extend(_build_model(build_threaded, exeroot, clm_config_opts, incroot, complist,
                                 lid, caseroot, cimeroot, compiler))
 
-    if not sharedlib_only:
         # in case component build scripts updated the xml files, update the case object
         case.read_xml()
         post_build(case, logs)