Merge pull request #4609 from ESMCI/fix-model-rebuild

Refactors how/when rebuilds are required

Refactors check_lockedfiles
Updates xmlchange to use check_lockedfiles
Adds ability for components to define when a rebuild is required
Define custom component triggers by adding REBUILD_TRIGGER_{comp} with comma separated list of variables to config_component.xml, e.g. REBUILD_TRIGGER_CPL=NTASKS would require a rebuild if NTASKS changes for the cpl component
Removes warning/error when pelayout is changed
To add old behavior define REBUILD_TRIGGER_CPL with a value NTASKS,NTHRDS,NINST to couplers config_component.xml
Test suite: n/a
Test baseline: n/a
Test namelist changes: n/a
Test status: n/a

Fixes n/a
User interface changes? n/a
Update gh-pages html (Y/N)?:

.pre-commit-config.yaml

@@ -10,6 +10,8 @@ repos:
         exclude: doc/
       - id: trailing-whitespace
         exclude: doc/
+      - id: debug-statements
+        exclude: doc/|CIME/utils.py
   - repo: https://github.com/psf/black
     rev: 22.3.0
     hooks:

CIME/SystemTests/system_tests_common.py

@@ -131,10 +131,10 @@ def _init_locked_files(self, caseroot, expected):
         env_run.xml file. If it does exist, restore values changed in a previous
         run of the test.
         """
-        if is_locked("env_run.orig.xml"):
+        if is_locked("env_run.orig.xml", caseroot):
             self.compare_env_run(expected=expected)
         elif os.path.isfile(os.path.join(caseroot, "env_run.xml")):
-            lock_file("env_run.xml", caseroot=caseroot, newname="env_run.orig.xml")
+            lock_file("env_run.xml", caseroot, newname="env_run.orig.xml")
 
     def _resetup_case(self, phase, reset=False):
         """

CIME/Tools/check_case

@@ -21,6 +21,7 @@ from standard_script_setup import *
 
 from CIME.utils import expect
 from CIME.case import Case
+from CIME.locked_files import check_lockedfiles
 
 import argparse
 
@@ -45,8 +46,10 @@ def _main_func(description):
     parse_command_line(sys.argv, description)
 
     with Case(read_only=False, record=True) as case:
-        case.check_lockedfiles()
+        check_lockedfiles(case)
+
         case.create_namelists()
+
         build_complete = case.get_value("BUILD_COMPLETE")
 
     if not build_complete:

CIME/Tools/check_lockedfiles

@@ -5,6 +5,7 @@ This script compares xml files
 
 from standard_script_setup import *
 from CIME.case import Case
+from CIME.locked_files import check_lockedfiles
 
 
 def parse_command_line(args, description):
@@ -38,7 +39,7 @@ def _main_func(description):
     caseroot = parse_command_line(sys.argv, description)
 
     with Case(case_root=caseroot, read_only=True) as case:
-        case.check_lockedfiles()
+        check_lockedfiles(case)
 
 
 if __name__ == "__main__":

CIME/Tools/xmlchange

@@ -55,6 +55,7 @@ from CIME.utils import (
     get_batch_script_for_job,
 )
 from CIME.case import Case
+from CIME.locked_files import check_lockedfiles
 
 import re
 
@@ -223,32 +224,10 @@ def xmlchange_single_value(
         result = case.set_value(
             xmlid, xmlval, subgroup, ignore_type=force, return_file=True
         )
-        expect(result is not None, 'No variable "%s" found' % xmlid)
-        filename = result[1]
-
-        setup_already_run = os.path.exists(
-            get_batch_script_for_job(case.get_primary_job())
-        )
-        build_already_run = case.get_value("BUILD_COMPLETE")
-
-        if filename.endswith("env_build.xml") and build_already_run:
-            logger.info(
-                """For your changes to take effect, run:
-./case.build --clean-all
-./case.build"""
-            )
-
-        elif filename.endswith("env_mach_pes.xml"):
-            if setup_already_run:
-                logger.info(
-                    "For your changes to take effect, run:\n./case.setup --reset"
-                )
-            if build_already_run:
-                if not setup_already_run:
-                    logger.info("For your changes to take effect, run:")
 
-                logger.info("./case.build --clean-all\n./case.build")
+        expect(result is not None, 'No variable "%s" found' % xmlid)
 
+        check_lockedfiles(case, skip=["env_case"])
     else:
         logger.warning("'%s' = '%s'", xmlid, xmlval)
 

CIME/XML/env_batch.py

@@ -17,7 +17,6 @@
     format_time,
     add_flag_to_cmd,
 )
-from CIME.locked_files import lock_file, unlock_file
 from collections import OrderedDict
 import stat, re, math
 import pathlib
@@ -190,13 +189,19 @@ def set_batch_system(self, batchobj, batch_system_type=None):
 
         if batchobj.batch_system_node is not None:
             self.add_child(self.copy(batchobj.batch_system_node))
+
         if batchobj.machine_node is not None:
             self.add_child(self.copy(batchobj.machine_node))
+
+        from CIME.locked_files import lock_file, unlock_file
+
         if os.path.exists(os.path.join(self._caseroot, "LockedFiles", "env_batch.xml")):
-            unlock_file(os.path.basename(batchobj.filename), caseroot=self._caseroot)
+            unlock_file(os.path.basename(batchobj.filename), self._caseroot)
+
         self.set_value("BATCH_SYSTEM", batch_system_type)
+
         if os.path.exists(os.path.join(self._caseroot, "LockedFiles")):
-            lock_file(os.path.basename(batchobj.filename), caseroot=self._caseroot)
+            lock_file(os.path.basename(batchobj.filename), self._caseroot)
 
     def get_job_overrides(self, job, case):
         env_workflow = case.get_env("workflow")

CIME/build.py

@@ -20,7 +20,7 @@
     import_from_file,
 )
 from CIME.config import Config
-from CIME.locked_files import lock_file, unlock_file
+from CIME.locked_files import lock_file, unlock_file, check_lockedfiles
 from CIME.XML.files import Files
 
 logger = logging.getLogger(__name__)
@@ -1026,7 +1026,7 @@ def _clean_impl(case, cleanlist, clean_all, clean_depends):
             run_cmd_no_fail(clean_cmd)
 
     # unlink Locked files directory
-    unlock_file("env_build.xml")
+    unlock_file("env_build.xml", case.get_value("CASEROOT"))
 
     # reset following values in xml files
     case.set_value("SMP_BUILD", str(0))
@@ -1076,7 +1076,7 @@ def _case_build_impl(
 
     comp_classes = case.get_values("COMP_CLASSES")
 
-    case.check_lockedfiles(skip="env_batch")
+    check_lockedfiles(case, skip="env_batch")
 
     # Retrieve relevant case data
     # This environment variable gets set for cesm Make and
@@ -1292,7 +1292,7 @@ def post_build(case, logs, build_complete=False, save_build_provenance=True):
 
         case.flush()
 
-        lock_file("env_build.xml", caseroot=case.get_value("CASEROOT"))
+        lock_file("env_build.xml", case.get_value("CASEROOT"))
 
 
 ###############################################################################

CIME/case/case.py

@@ -90,11 +90,6 @@ class Case(object):
     )
     from CIME.case.case_run import case_run
     from CIME.case.case_cmpgen_namelists import case_cmpgen_namelists
-    from CIME.case.check_lockedfiles import (
-        check_lockedfile,
-        check_lockedfiles,
-        check_pelayouts_require_rebuild,
-    )
     from CIME.case.preview_namelists import create_dirs, create_namelists
     from CIME.case.check_input_data import (
         check_all_input_data,

CIME/case/case_run.py

@@ -7,6 +7,7 @@
 from CIME.utils import run_sub_or_cmd, append_status, safe_copy, model_log, CIMEError
 from CIME.utils import batch_jobid, is_comp_standalone
 from CIME.get_timing import get_timing
+from CIME.locked_files import check_lockedfiles
 
 import shutil, time, sys, os, glob
 
@@ -34,10 +35,14 @@ def _pre_run_check(case, lid, skip_pnl=False, da_cycle=0):
 
     # check for locked files, may impact BUILD_COMPLETE
     skip = None
+
     if case.get_value("EXTERNAL_WORKFLOW"):
         skip = "env_batch"
-    case.check_lockedfiles(skip=skip)
+
+    check_lockedfiles(case, skip=skip)
+
     logger.debug("check_lockedfiles OK")
+
     build_complete = case.get_value("BUILD_COMPLETE")
 
     # check that build is done

CIME/case/case_setup.py

@@ -22,7 +22,7 @@
 )
 from CIME.utils import batch_jobid
 from CIME.test_status import *
-from CIME.locked_files import unlock_file, lock_file
+from CIME.locked_files import unlock_file, lock_file, check_lockedfiles
 
 import errno, shutil
 
@@ -348,13 +348,15 @@ def _case_setup_impl(
             case.set_value("BUILD_THREADED", case.get_build_threaded())
 
         else:
-            case.check_pelayouts_require_rebuild(models)
+            caseroot = case.get_value("CASEROOT")
 
-            unlock_file("env_build.xml")
-            unlock_file("env_batch.xml")
+            unlock_file("env_build.xml", caseroot)
+
+            unlock_file("env_batch.xml", caseroot)
 
             case.flush()
-            case.check_lockedfiles()
+
+            check_lockedfiles(case, skip=["env_build", "env_mach_pes"])
 
             case.initialize_derived_attributes()
 
@@ -404,9 +406,14 @@ def _case_setup_impl(
             # Make a copy of env_mach_pes.xml in order to be able
             # to check that it does not change once case.setup is invoked
             case.flush()
+
             logger.debug("at copy TOTALPES = {}".format(case.get_value("TOTALPES")))
-            lock_file("env_mach_pes.xml")
-            lock_file("env_batch.xml")
+
+            caseroot = case.get_value("CASEROOT")
+
+            lock_file("env_mach_pes.xml", caseroot)
+
+            lock_file("env_batch.xml", caseroot)
 
         # Create user_nl files for the required number of instances
         if not os.path.exists("user_nl_cpl"):

CIME/case/case_submit.py

@@ -9,7 +9,12 @@
 import configparser
 from CIME.XML.standard_module_setup import *
 from CIME.utils import expect, run_and_log_case_status, CIMEError, get_time_in_seconds
-from CIME.locked_files import unlock_file, lock_file
+from CIME.locked_files import (
+    unlock_file,
+    lock_file,
+    check_lockedfile,
+    check_lockedfiles,
+)
 from CIME.test_status import *
 
 logger = logging.getLogger(__name__)
@@ -95,15 +100,14 @@ def _submit(
         batch_system = env_batch.get_batch_system_type()
 
     if batch_system != case.get_value("BATCH_SYSTEM"):
-        unlock_file(os.path.basename(env_batch.filename), caseroot=caseroot)
+        unlock_file(os.path.basename(env_batch.filename), caseroot)
+
         case.set_value("BATCH_SYSTEM", batch_system)
 
     env_batch_has_changed = False
     if not external_workflow:
         try:
-            case.check_lockedfile(
-                os.path.basename(env_batch.filename), caseroot=caseroot
-            )
+            check_lockedfile(case, os.path.basename(env_batch.filename))
         except:
             env_batch_has_changed = True
 
@@ -116,8 +120,10 @@ def _submit(
 """
         )
         env_batch.make_all_batch_files(case)
+
     case.flush()
-    lock_file(os.path.basename(env_batch.filename), caseroot=caseroot)
+
+    lock_file(os.path.basename(env_batch.filename), caseroot)
 
     if resubmit:
         # This is a resubmission, do not reinitialize test values
@@ -143,7 +149,7 @@ def _submit(
 
         env_batch_has_changed = False
         try:
-            case.check_lockedfile(os.path.basename(env_batch.filename))
+            check_lockedfile(case, os.path.basename(env_batch.filename))
         except CIMEError:
             env_batch_has_changed = True
 
@@ -157,10 +163,12 @@ def _submit(
             )
             env_batch.make_all_batch_files(case)
 
-        unlock_file(os.path.basename(env_batch.filename), caseroot=caseroot)
-        lock_file(os.path.basename(env_batch.filename), caseroot=caseroot)
+        unlock_file(os.path.basename(env_batch.filename), caseroot)
+
+        lock_file(os.path.basename(env_batch.filename), caseroot)
 
         case.check_case(skip_pnl=skip_pnl, chksum=chksum)
+
         if job == case.get_primary_job():
             case.check_DA_settings()
 
@@ -287,7 +295,8 @@ def submit(
 
 
 def check_case(self, skip_pnl=False, chksum=False):
-    self.check_lockedfiles()
+    check_lockedfiles(self)
+
     if not skip_pnl:
         self.create_namelists()  # Must be called before check_all_input_data