Merge pull request #1919 from billsacks/compare_two_shorter_path

Shorter paths for case2 using system_tests_compare_two
The recently-changed version of system_tests_compare_two nested case2's
bld and run directories under
$CIME_OUTPUT_ROOT/TESTNAME/case2/TESTNAME. Since the TESTNAME is often
long, this was leading to very long paths to some things in the bld
directory. This was causing some compilation errors.

With this PR, case2's bld and run directories appear in
$CIME_OUTPUT_ROOT/TESTNAME/case2/bld and
$CIME_OUTPUT_ROOT/TESTNAME/case2/run. This shortens the bld directory
paths enough to get some formerly-failing builds to pass. The change in
run directory is just for consistency.

I was hoping to do this just via setting EXEROOT and RUNDIR in
system_tests_compare_two, to avoid cluttering up create_clone with
arguments that are just needed for tests. But it turns out that we need
to do this in create_clone, because create_clone calls case_setup, which
creates the EXEROOT and RUNDIR directories.

Test suite: scripts_regression_tests on cheyenne
Also confirmed that the following tests build successfully:
ERP.f10_f10_musgs.I1850Clm45BgcCru.yellowstone_pgi
ERP_P60x2_Lm36.f10_f10_musgs.I2000Clm50BgcCrop.yellowstone_intel.clm-clm50cropIrrigMonth_interp
Test baseline: N/A
Test namelist changes: none
Test status: bit for bit

Fixes #1914

User interface changes?: none

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

Code review: jedwards

scripts/lib/CIME/SystemTests/system_tests_compare_two.py

@@ -179,10 +179,13 @@ def _case_two_custom_postrun_action(self):
     def build_phase(self, sharedlib_only=False, model_only=False):
         if self._separate_builds:
             self._activate_case1()
-            sharedlibroot = self._case1.get_value("SHAREDLIBROOT")
             self.build_indv(sharedlib_only=sharedlib_only, model_only=model_only)
             self._activate_case2()
-            self._case.set_value("SHAREDLIBROOT", sharedlibroot)
+            # Although we're doing separate builds, it still makes sense
+            # to share the sharedlibroot area with case1 so we can reuse
+            # pieces of the build from there.
+            self._case2.set_value("SHAREDLIBROOT",
+                                  self._case1.get_value("SHAREDLIBROOT"))
             self.build_indv(sharedlib_only=sharedlib_only, model_only=model_only)
         else:
             self._activate_case1()
@@ -258,8 +261,7 @@ def _get_caseroot2(self):
         """
         Determines and returns caseroot for case2
 
-        Assumes that self._case1 is already set to point to the case1 object,
-        and that self._run_two_suffix is already set.
+        Assumes that self._case1 is already set to point to the case1 object
         """
         casename2 = self._case1.get_value("CASE")
         caseroot1 = self._case1.get_value("CASEROOT")
@@ -269,6 +271,49 @@ def _get_caseroot2(self):
 
         return caseroot2
 
+    def _get_output_root2(self):
+        """
+        Determines and returns cime_output_root for case2
+
+        Assumes that self._case1 is already set to point to the case1 object
+        """
+        # Since case 2 has the same name as case1 its CIME_OUTPUT_ROOT must also be different
+        output_root2 = os.path.join(self._case1.get_value("CIME_OUTPUT_ROOT"),
+                                    self._case1.get_value("CASE"), "case2")
+        return output_root2
+
+    def _get_case2_exeroot(self):
+        """
+        Gets exeroot for case2.
+
+        Returns None if we should use the default value of exeroot.
+        """
+        if self._separate_builds:
+            # Put the case2 bld directory directly under the case2
+            # CIME_OUTPUT_ROOT, rather than following the typical
+            # practice of putting it under CIME_OUTPUT_ROOT/CASENAME,
+            # because the latter leads to too-long paths that make some
+            # compilers fail.
+            #
+            # This only works because case2's CIME_OUTPUT_ROOT is unique
+            # to this case. (If case2's CIME_OUTPUT_ROOT were in some
+            # more generic location, then this would result in its bld
+            # directory being inadvertently shared with other tests.)
+            case2_exeroot = os.path.join(self._get_output_root2(), "bld")
+        else:
+            # Use default exeroot
+            case2_exeroot = None
+        return case2_exeroot
+
+    def _get_case2_rundir(self):
+        """
+        Gets rundir for case2.
+        """
+        # Put the case2 run directory alongside its bld directory for
+        # consistency. (See notes about EXEROOT in _get_case2_exeroot.)
+        case2_rundir = os.path.join(self._get_output_root2(), "run")
+        return case2_rundir
+
     def _setup_cases_if_not_yet_done(self):
         """
         Determines if case2 already exists on disk. If it does, this method
@@ -298,13 +343,12 @@ def _setup_cases_if_not_yet_done(self):
             self._case2 = self._case_from_existing_caseroot(self._caseroot2)
         else:
             try:
-                # Since case 2 has the same name as case1 its CIME_OUTPUT_ROOT must also be different
-                case2_output_root = os.path.join(self._case1.get_value("CIME_OUTPUT_ROOT"),
-                                                  self._case1.get_value("CASE"), "case2")
                 self._case2 = self._case1.create_clone(
                     self._caseroot2,
                     keepexe = not self._separate_builds,
-                    cime_output_root = case2_output_root)
+                    cime_output_root = self._get_output_root2(),
+                    exeroot = self._get_case2_exeroot(),
+                    rundir = self._get_case2_rundir())
                 self._setup_cases()
             except:
                 # If a problem occurred in setting up the test cases, it's

scripts/lib/CIME/case.py

@@ -1395,9 +1395,11 @@ def create(self, casename, srcroot, compset_name, grid_name,
                     logger.warn("Leaving broken case dir {}".format(self._caseroot))
 
             raise
-    def create_clone(self, newcase, keepexe=False, mach_dir=None, project=None, cime_output_root=None,
-                 user_mods_dir=None):
+    def create_clone(self, newcase, keepexe=False, mach_dir=None, project=None,
+                     cime_output_root=None, exeroot=None, rundir=None,
+                     user_mods_dir=None):
         """ moved to case_clone """
         return create_case_clone(self, newcase, keepexe=keepexe, mach_dir=mach_dir,
-                          project=project, cime_output_root=cime_output_root,
-                          user_mods_dir=user_mods_dir)
+                                 project=project, cime_output_root=cime_output_root,
+                                 exeroot=exeroot, rundir=rundir,
+                                 user_mods_dir=user_mods_dir)

scripts/lib/CIME/case_clone.py

@@ -9,10 +9,15 @@
 logger = logging.getLogger(__name__)
 
 
-def create_case_clone(case, newcase, keepexe=False, mach_dir=None, project=None, cime_output_root=None,
-                 user_mods_dir=None):
+def create_case_clone(case, newcase, keepexe=False, mach_dir=None, project=None,
+                      cime_output_root=None, exeroot=None, rundir=None,
+                      user_mods_dir=None):
     """
     Create a case clone
+
+    If exeroot or rundir are provided (not None), sets these directories
+    to the given paths; if not provided, uses default values for these
+    directories. It is an error to provide exeroot if keepexe is True.
     """
     if cime_output_root is None:
         cime_output_root = case.get_value("CIME_OUTPUT_ROOT")
@@ -46,7 +51,7 @@ def create_case_clone(case, newcase, keepexe=False, mach_dir=None, project=None,
     # try to make the new output directory and raise an exception
     # on any error other than directory already exists.
     if os.path.isdir(cime_output_root):
-        expect(os.access(cime_output_root, os.W_OK), "Directory {} is not writable"
+        expect(os.access(cime_output_root, os.W_OK), "Directory {} is not writable "
                "by this user.  Use the --cime-output-root flag to provide a writable "
                "scratch directory".format(cime_output_root))
     else:
@@ -72,6 +77,14 @@ def create_case_clone(case, newcase, keepexe=False, mach_dir=None, project=None,
     if mach_dir is not None:
         newcase.set_value("MACHDIR", mach_dir)
 
+    # set exeroot and rundir if requested
+    if exeroot is not None:
+        expect(not keepexe, "create_case_clone: if keepexe is True, "
+               "then exeroot cannot be set")
+        newcase.set_value("EXEROOT", exeroot)
+    if rundir is not None:
+        newcase.set_value("RUNDIR", rundir)
+
     # Set project id
     # Note: we do not just copy this from the clone because it seems likely that
     # users will want to change this sometimes, especially when cloning another

scripts/lib/CIME/tests/SystemTests/test_system_tests_compare_two.py

@@ -73,6 +73,7 @@ def __init__(self,
                  case1,
                  run_one_suffix = 'base',
                  run_two_suffix = 'test',
+                 separate_builds = False,
                  multisubmit = False,
                  case2setup_raises_exception = False,
                  run_one_should_pass = True,
@@ -88,6 +89,7 @@ def __init__(self,
             run_one_suffix (str, optional): Suffix used for first run. Defaults
                 to 'base'. Currently MUST be 'base'.
             run_two_suffix (str, optional): Suffix used for the second run. Defaults to 'test'.
+            separate_builds (bool, optional): Passed to SystemTestsCompareTwo.__init__
             multisubmit (bool, optional): Passed to SystemTestsCompareTwo.__init__
             case2setup_raises_exception (bool, optional): If True, then the call
                 to _case_two_setup will raise an exception. Default is False.
@@ -110,7 +112,7 @@ def __init__(self,
         SystemTestsCompareTwo.__init__(
             self,
             case1,
-            separate_builds = False,
+            separate_builds = separate_builds,
             run_two_suffix = run_two_suffix,
             multisubmit = multisubmit)
 
@@ -300,6 +302,23 @@ def test_setup(self):
         self.assertEqual('case2val',
                          mytest._case2.get_value('var_set_in_setup'))
 
+    def test_setup_separate_builds_sharedlibroot(self):
+        # If we're using separate_builds, the two cases should still use
+        # the same sharedlibroot
+
+        # Setup
+        case1root, _ = self.get_caseroots()
+        case1 = CaseFake(case1root)
+        case1.set_value("SHAREDLIBROOT", os.path.join(case1root, "sharedlibroot"))
+
+        # Exercise
+        mytest = SystemTestsCompareTwoFake(case1,
+                                           separate_builds = True)
+
+        # Verify
+        self.assertEqual(case1.get_value("SHAREDLIBROOT"),
+                         mytest._case2.get_value("SHAREDLIBROOT"))
+
     def test_setup_case2_exists(self):
         # If case2 already exists, then setup code should not be called
 

scripts/lib/CIME/tests/case_fake.py

@@ -20,7 +20,12 @@ def __init__(self, case_root, create_case_root=True):
             os.makedirs(case_root)
         self.set_value('CASEROOT', case_root)
         casename = os.path.basename(case_root)
-        self.set_value('CIME_OUTPUT_ROOT','/tmp')
+        # Typically, CIME_OUTPUT_ROOT is independent of the case. Here,
+        # we nest it under CASEROOT so that (1) tests don't interfere
+        # with each other; (2) a cleanup that removes CASEROOT will also
+        # remove CIME_OUTPUT_ROOT.
+        self.set_value('CIME_OUTPUT_ROOT',
+                       os.path.join(case_root, 'CIME_OUTPUT_ROOT'))
         self.set_value('CASE', casename)
         self.set_value('CASEBASEID', casename)
         self.set_value('RUN_TYPE', 'startup')
@@ -64,7 +69,8 @@ def copy(self, newcasename, newcaseroot):
 
         return newcase
 
-    def create_clone(self, newcase, keepexe=False, mach_dir=None, project=None, cime_output_root=None):
+    def create_clone(self, newcase, keepexe=False, mach_dir=None, project=None,
+                     cime_output_root=None, exeroot=None, rundir=None):
         # Need to disable unused-argument checking: keepexe is needed to match
         # the interface of Case, but is not used in this fake implementation
         #
@@ -77,13 +83,23 @@ def create_clone(self, newcase, keepexe=False, mach_dir=None, project=None, cime
             newcase (str): full path to the new case. This directory should not
                 already exist; it will be created
             keepexe (bool, optional): Ignored
+            mach_dir (str, optional): Ignored
+            project (str, optional): Ignored
+            cime_output_root (str, optional): New CIME_OUTPUT_ROOT for the clone
+            exeroot (str, optional): Ignored (because exeroot isn't used
+                in this fake case implementation)
+            rundir (str, optional): New RUNDIR for the clone
 
         Returns the clone case object
         """
         newcaseroot = os.path.abspath(newcase)
         newcasename = os.path.basename(newcase)
         os.makedirs(newcaseroot)
         clone = self.copy(newcasename = newcasename, newcaseroot = newcaseroot)
+        if cime_output_root is not None:
+            self.set_value('CIME_OUTPUT_ROOT', cime_output_root)
+        if rundir is not None:
+            self.set_value('RUNDIR', rundir)
 
         return clone
 

scripts/tests/scripts_regression_tests.py

@@ -80,7 +80,7 @@ def test_unittests(self):
         #
         # This is analogous to running:
         #     python -m unittest discover -s CIME/tests -t .
-        # from cime/utils/python
+        # from cime/scripts/lib
         #
         # Yes, that means we have a bunch of unit tests run from this one unit
         # test.