From f18282a0510cc133d77e29d4748a319892b375b2 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Tue, 20 Aug 2024 17:21:13 -0600 Subject: [PATCH 1/9] fixes issues with REST_N in tests by removing from config_tests --- CIME/SystemTests/ers.py | 14 +---- CIME/SystemTests/system_tests_common.py | 68 +++++++++++++++++++++++++ CIME/XML/env_test.py | 2 +- CIME/case/case_submit.py | 1 + 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/CIME/SystemTests/ers.py b/CIME/SystemTests/ers.py index bebed8f04c4..f7d017a14bd 100644 --- a/CIME/SystemTests/ers.py +++ b/CIME/SystemTests/ers.py @@ -14,19 +14,9 @@ def __init__(self, case, **kwargs): initialize an object interface to the ERS system test """ SystemTestsCommon.__init__(self, case, **kwargs) - + def _ers_first_phase(self): - stop_n = self._case.get_value("STOP_N") - stop_option = self._case.get_value("STOP_OPTION") - rest_n = self._case.get_value("REST_N") - expect(stop_n > 0, "Bad STOP_N: {:d}".format(stop_n)) - - expect(stop_n > 2, "ERROR: stop_n value {:d} too short".format(stop_n)) - logger.info( - "doing an {0} {1} initial test with restart file at {2} {1}".format( - str(stop_n), stop_option, str(rest_n) - ) - ) + self._set_restart_interval() self.run_indv() def _ers_second_phase(self): diff --git a/CIME/SystemTests/system_tests_common.py b/CIME/SystemTests/system_tests_common.py index 13057dd52e9..44a2818f1b6 100644 --- a/CIME/SystemTests/system_tests_common.py +++ b/CIME/SystemTests/system_tests_common.py @@ -118,6 +118,74 @@ def __init__( self._user_separate_builds = False self._expected_num_cmp = None + def _set_restart_interval(self): + stop_n = self._case.get_value("STOP_N") + stop_option = self._case.get_value("STOP_OPTION") + self._case.set_value("REST_OPTION", stop_option) + # We need to make sure the run is long enough and to set REST_N to a + # value that makes sense for all components + maxncpl = 10000 + minncpl = 0 + maxcomp = None + for comp in self._case.get_values("COMP_CLASSES"): + if comp == "CPL": + continue + compname = self._case.get_value("COMP_{}".format(comp)) + + # ignore stub components in this test. + if compname == "s{}".format(comp.lower()): + ncpl = None + else: + ncpl = self._case.get_value("{}_NCPL".format(comp)) + + if ncpl and maxncpl > ncpl: + maxncpl = ncpl + maxcomp = comp + if ncpl and minncpl < ncpl: + minncpl = ncpl + + ncpl_base_period = self._case.get_value("NCPL_BASE_PERIOD") + if ncpl_base_period == "hour": + coupling_secs = 3600 / maxncpl + timestep = 3600 / minncpl + elif ncpl_base_period == "day": + coupling_secs = 86400 / maxncpl + timestep = 86400 / minncpl + elif ncpl_base_period == "year": + coupling_secs = 31536000 / maxncpl + timestep = 31536000 / minncpl + elif ncpl_base_period == "decade": + coupling_secs = 315360000 / maxncpl + timestep = 315360000 / minncpl + + # Convert stop_n to units of coupling intervals + factor = 1 + if stop_option == "nsteps": + factor = timestep + elif stop_option == "nminutes": + factor = 60 + elif stop_option == "nhours": + factor = 3600 + elif stop_option == "ndays": + factor = 86400 + elif stop_option == "nyears": + factor = 315360000 + else: + expect(False, f"stop_option {stop_option} not available for this test") + + stop_n = int(stop_n * factor // coupling_secs) + rest_n = int((stop_n//2 + 1) * coupling_secs / factor) + + expect(stop_n > 0, "Bad STOP_N: {:d}".format(stop_n)) + + expect(stop_n > 2, "ERROR: stop_n value {:d} too short".format(stop_n)) + logger.info( + "doing an {0} {1} initial test with restart file at {2} {1}".format( + str(stop_n), stop_option, str(rest_n) + ) + ) + self._case.set_value("REST_N", rest_n) + def _init_environment(self, caseroot): """ Do initializations of environment variables that are needed in __init__ diff --git a/CIME/XML/env_test.py b/CIME/XML/env_test.py index 9dbe3615eff..e2e83729959 100644 --- a/CIME/XML/env_test.py +++ b/CIME/XML/env_test.py @@ -4,7 +4,7 @@ from CIME.XML.standard_module_setup import * from CIME.XML.env_base import EnvBase -from CIME.utils import convert_to_type +from CIME.utils import convert_to_type, expect logger = logging.getLogger(__name__) diff --git a/CIME/case/case_submit.py b/CIME/case/case_submit.py index 037a08f0615..25bc783ed1b 100644 --- a/CIME/case/case_submit.py +++ b/CIME/case/case_submit.py @@ -233,6 +233,7 @@ def submit( caseroot = self.get_value("CASEROOT") if self.get_value("TEST"): casebaseid = self.get_value("CASEBASEID") + self.set_initial_test_values() # This should take care of the race condition where the submitted job # begins immediately and tries to set RUN phase. We proactively assume # a passed SUBMIT phase. If this state is already PASS, don't set it again From 081969f9886a2e541e2479d62822da04a2c3de25 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 10:00:40 -0600 Subject: [PATCH 2/9] ers and err --- CIME/SystemTests/err.py | 3 ++- CIME/SystemTests/ers.py | 7 +++---- CIME/SystemTests/system_tests_common.py | 3 ++- CIME/data/config/config_tests.xml | 1 - 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CIME/SystemTests/err.py b/CIME/SystemTests/err.py index 355ddd5d390..ebfc84eb94e 100644 --- a/CIME/SystemTests/err.py +++ b/CIME/SystemTests/err.py @@ -28,7 +28,8 @@ def __init__(self, case, **kwargs): # pylint: disable=super-init-not-called def _case_one_setup(self): super(ERR, self)._case_one_setup() self._case.set_value("DOUT_S", True) - + self._rest_n = self._set_restart_interval() + def _case_two_setup(self): super(ERR, self)._case_two_setup() self._case.set_value("DOUT_S", False) diff --git a/CIME/SystemTests/ers.py b/CIME/SystemTests/ers.py index f7d017a14bd..07ba1818f1e 100644 --- a/CIME/SystemTests/ers.py +++ b/CIME/SystemTests/ers.py @@ -16,19 +16,18 @@ def __init__(self, case, **kwargs): SystemTestsCommon.__init__(self, case, **kwargs) def _ers_first_phase(self): - self._set_restart_interval() + self._rest_n = self._set_restart_interval() self.run_indv() def _ers_second_phase(self): stop_n = self._case.get_value("STOP_N") stop_option = self._case.get_value("STOP_OPTION") - rest_n = int(stop_n / 2 + 1) - stop_new = stop_n - rest_n + stop_new = stop_n - self._rest_n expect( stop_new > 0, "ERROR: stop_n value {:d} too short {:d} {:d}".format( - stop_new, stop_n, rest_n + stop_new, stop_n, self._rest_n ), ) rundir = self._case.get_value("RUNDIR") diff --git a/CIME/SystemTests/system_tests_common.py b/CIME/SystemTests/system_tests_common.py index 44a2818f1b6..b6a0b65790f 100644 --- a/CIME/SystemTests/system_tests_common.py +++ b/CIME/SystemTests/system_tests_common.py @@ -185,7 +185,8 @@ def _set_restart_interval(self): ) ) self._case.set_value("REST_N", rest_n) - + return rest_n + def _init_environment(self, caseroot): """ Do initializations of environment variables that are needed in __init__ diff --git a/CIME/data/config/config_tests.xml b/CIME/data/config/config_tests.xml index 0352b5207ca..46f48856e91 100644 --- a/CIME/data/config/config_tests.xml +++ b/CIME/data/config/config_tests.xml @@ -320,7 +320,6 @@ NODEFAIL Tests restart upon detected node failure. Generates fake failu 1 ndays 7 - $STOP_N / 2 + 1 $STOP_OPTION $STOP_N $STOP_OPTION From e4980a23dc32767abdbbe501cd22096c31223f69 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 12:25:31 -0600 Subject: [PATCH 3/9] address the rest of the restart tests --- CIME/SystemTests/err.py | 1 - CIME/SystemTests/restart_tests.py | 3 ++- CIME/SystemTests/system_tests_common.py | 3 ++- CIME/case/case.py | 2 ++ CIME/data/config/config_tests.xml | 2 -- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CIME/SystemTests/err.py b/CIME/SystemTests/err.py index ebfc84eb94e..22329dc5b0e 100644 --- a/CIME/SystemTests/err.py +++ b/CIME/SystemTests/err.py @@ -28,7 +28,6 @@ def __init__(self, case, **kwargs): # pylint: disable=super-init-not-called def _case_one_setup(self): super(ERR, self)._case_one_setup() self._case.set_value("DOUT_S", True) - self._rest_n = self._set_restart_interval() def _case_two_setup(self): super(ERR, self)._case_two_setup() diff --git a/CIME/SystemTests/restart_tests.py b/CIME/SystemTests/restart_tests.py index 5faf2252d1b..f0339d3ccc2 100644 --- a/CIME/SystemTests/restart_tests.py +++ b/CIME/SystemTests/restart_tests.py @@ -34,7 +34,8 @@ def __init__( def _case_one_setup(self): stop_n = self._case1.get_value("STOP_N") expect(stop_n >= 3, "STOP_N must be at least 3, STOP_N = {}".format(stop_n)) - + self._set_restart_interval() + def _case_two_setup(self): rest_n = self._case1.get_value("REST_N") stop_n = self._case1.get_value("STOP_N") diff --git a/CIME/SystemTests/system_tests_common.py b/CIME/SystemTests/system_tests_common.py index b6a0b65790f..4101a291377 100644 --- a/CIME/SystemTests/system_tests_common.py +++ b/CIME/SystemTests/system_tests_common.py @@ -117,7 +117,8 @@ def __init__( self._dry_run = False self._user_separate_builds = False self._expected_num_cmp = None - + self._rest_n = None + def _set_restart_interval(self): stop_n = self._case.get_value("STOP_N") stop_option = self._case.get_value("STOP_OPTION") diff --git a/CIME/case/case.py b/CIME/case/case.py index 7aab433b9a1..f65884b89ad 100644 --- a/CIME/case/case.py +++ b/CIME/case/case.py @@ -169,6 +169,7 @@ def __init__(self, case_root=None, read_only=True, record=False, non_local=False # Command Line user_mods are handled seperately # Derived attributes + self.mem_per_node = None self.thread_count = None self.total_tasks = None self.tasks_per_node = None @@ -265,6 +266,7 @@ def initialize_derived_attributes(self): self.tasks_per_node = env_mach_pes.get_tasks_per_node( self.total_tasks, self.thread_count ) + self.mem_per_node = 230 / max_mpitasks_per_node * self.tasks_per_node self.num_nodes, self.spare_nodes = env_mach_pes.get_total_nodes( self.total_tasks, self.thread_count diff --git a/CIME/data/config/config_tests.xml b/CIME/data/config/config_tests.xml index 46f48856e91..0d5a4b86c68 100644 --- a/CIME/data/config/config_tests.xml +++ b/CIME/data/config/config_tests.xml @@ -332,7 +332,6 @@ NODEFAIL Tests restart upon detected node failure. Generates fake failu 1 ndays 7 - $STOP_N / 2 + 1 $STOP_OPTION $STOP_N $STOP_OPTION @@ -539,7 +538,6 @@ NODEFAIL Tests restart upon detected node failure. Generates fake failu nsteps $ATM_NCPL 11 - $STOP_N / 2 + 1 $STOP_OPTION $STOP_N $STOP_OPTION From d489e1c8e903edda06940497d15fe41ed7a2c926 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 12:44:12 -0600 Subject: [PATCH 4/9] cleanup --- CIME/SystemTests/err.py | 2 +- CIME/SystemTests/ers.py | 2 +- CIME/XML/env_test.py | 2 +- CIME/case/case.py | 4 +--- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/CIME/SystemTests/err.py b/CIME/SystemTests/err.py index 22329dc5b0e..355ddd5d390 100644 --- a/CIME/SystemTests/err.py +++ b/CIME/SystemTests/err.py @@ -28,7 +28,7 @@ def __init__(self, case, **kwargs): # pylint: disable=super-init-not-called def _case_one_setup(self): super(ERR, self)._case_one_setup() self._case.set_value("DOUT_S", True) - + def _case_two_setup(self): super(ERR, self)._case_two_setup() self._case.set_value("DOUT_S", False) diff --git a/CIME/SystemTests/ers.py b/CIME/SystemTests/ers.py index 07ba1818f1e..0e4bccd953a 100644 --- a/CIME/SystemTests/ers.py +++ b/CIME/SystemTests/ers.py @@ -14,7 +14,7 @@ def __init__(self, case, **kwargs): initialize an object interface to the ERS system test """ SystemTestsCommon.__init__(self, case, **kwargs) - + def _ers_first_phase(self): self._rest_n = self._set_restart_interval() self.run_indv() diff --git a/CIME/XML/env_test.py b/CIME/XML/env_test.py index e2e83729959..9dbe3615eff 100644 --- a/CIME/XML/env_test.py +++ b/CIME/XML/env_test.py @@ -4,7 +4,7 @@ from CIME.XML.standard_module_setup import * from CIME.XML.env_base import EnvBase -from CIME.utils import convert_to_type, expect +from CIME.utils import convert_to_type logger = logging.getLogger(__name__) diff --git a/CIME/case/case.py b/CIME/case/case.py index f65884b89ad..5373ab06683 100644 --- a/CIME/case/case.py +++ b/CIME/case/case.py @@ -169,7 +169,6 @@ def __init__(self, case_root=None, read_only=True, record=False, non_local=False # Command Line user_mods are handled seperately # Derived attributes - self.mem_per_node = None self.thread_count = None self.total_tasks = None self.tasks_per_node = None @@ -266,11 +265,10 @@ def initialize_derived_attributes(self): self.tasks_per_node = env_mach_pes.get_tasks_per_node( self.total_tasks, self.thread_count ) - self.mem_per_node = 230 / max_mpitasks_per_node * self.tasks_per_node - self.num_nodes, self.spare_nodes = env_mach_pes.get_total_nodes( self.total_tasks, self.thread_count ) + self.num_nodes += self.spare_nodes logger.debug( From 49a0f392eb9ac32108e949629432c96e763c0ee8 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 13:33:15 -0600 Subject: [PATCH 5/9] test pre-commit hook --- CIME/SystemTests/restart_tests.py | 3 +-- CIME/SystemTests/system_tests_common.py | 11 +++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/CIME/SystemTests/restart_tests.py b/CIME/SystemTests/restart_tests.py index f0339d3ccc2..4252739d326 100644 --- a/CIME/SystemTests/restart_tests.py +++ b/CIME/SystemTests/restart_tests.py @@ -1,6 +1,5 @@ """ Abstract class for restart tests - """ from CIME.SystemTests.system_tests_compare_two import SystemTestsCompareTwo @@ -35,7 +34,7 @@ def _case_one_setup(self): stop_n = self._case1.get_value("STOP_N") expect(stop_n >= 3, "STOP_N must be at least 3, STOP_N = {}".format(stop_n)) self._set_restart_interval() - + def _case_two_setup(self): rest_n = self._case1.get_value("REST_N") stop_n = self._case1.get_value("STOP_N") diff --git a/CIME/SystemTests/system_tests_common.py b/CIME/SystemTests/system_tests_common.py index 4101a291377..8bed4933179 100644 --- a/CIME/SystemTests/system_tests_common.py +++ b/CIME/SystemTests/system_tests_common.py @@ -1,6 +1,7 @@ """ Base class for CIME system tests """ + from CIME.XML.standard_module_setup import * from CIME.XML.env_run import EnvRun from CIME.XML.env_test import EnvTest @@ -118,7 +119,7 @@ def __init__( self._user_separate_builds = False self._expected_num_cmp = None self._rest_n = None - + def _set_restart_interval(self): stop_n = self._case.get_value("STOP_N") stop_option = self._case.get_value("STOP_OPTION") @@ -127,7 +128,6 @@ def _set_restart_interval(self): # value that makes sense for all components maxncpl = 10000 minncpl = 0 - maxcomp = None for comp in self._case.get_values("COMP_CLASSES"): if comp == "CPL": continue @@ -141,7 +141,6 @@ def _set_restart_interval(self): if ncpl and maxncpl > ncpl: maxncpl = ncpl - maxcomp = comp if ncpl and minncpl < ncpl: minncpl = ncpl @@ -175,8 +174,8 @@ def _set_restart_interval(self): expect(False, f"stop_option {stop_option} not available for this test") stop_n = int(stop_n * factor // coupling_secs) - rest_n = int((stop_n//2 + 1) * coupling_secs / factor) - + rest_n = int((stop_n // 2 + 1) * coupling_secs / factor) + expect(stop_n > 0, "Bad STOP_N: {:d}".format(stop_n)) expect(stop_n > 2, "ERROR: stop_n value {:d} too short".format(stop_n)) @@ -187,7 +186,7 @@ def _set_restart_interval(self): ) self._case.set_value("REST_N", rest_n) return rest_n - + def _init_environment(self, caseroot): """ Do initializations of environment variables that are needed in __init__ From 16d931260fa222f8d42da3c732b6dd625a206196 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 13:36:03 -0600 Subject: [PATCH 6/9] black reformat --- CIME/code_checker.py | 1 + 1 file changed, 1 insertion(+) diff --git a/CIME/code_checker.py b/CIME/code_checker.py index c0cba663208..6de5c14d34d 100644 --- a/CIME/code_checker.py +++ b/CIME/code_checker.py @@ -24,6 +24,7 @@ logger = logging.getLogger(__name__) + ############################################################################### def _run_pylint(all_files, interactive): ############################################################################### From a7823c87b0bab267409028d7211807f8936639f6 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 13:51:18 -0600 Subject: [PATCH 7/9] fix issue with mock case --- CIME/Servers/__init__.py | 8 ++++---- CIME/case/case_submit.py | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CIME/Servers/__init__.py b/CIME/Servers/__init__.py index 5cd14c0f89d..fb8307ba59d 100644 --- a/CIME/Servers/__init__.py +++ b/CIME/Servers/__init__.py @@ -1,9 +1,9 @@ # pylint: disable=import-error -from distutils.spawn import find_executable +from shutil import which -has_gftp = find_executable("globus-url-copy") -has_svn = find_executable("svn") -has_wget = find_executable("wget") +has_gftp = which("globus-url-copy") +has_svn = which("svn") +has_wget = which("wget") has_ftp = True try: from ftplib import FTP diff --git a/CIME/case/case_submit.py b/CIME/case/case_submit.py index 25bc783ed1b..8865d2311ce 100644 --- a/CIME/case/case_submit.py +++ b/CIME/case/case_submit.py @@ -233,7 +233,8 @@ def submit( caseroot = self.get_value("CASEROOT") if self.get_value("TEST"): casebaseid = self.get_value("CASEBASEID") - self.set_initial_test_values() + if os.path.exists(os.path.join(caseroot, "env_test.xml")): + self.set_initial_test_values() # This should take care of the race condition where the submitted job # begins immediately and tries to set RUN phase. We proactively assume # a passed SUBMIT phase. If this state is already PASS, don't set it again From 317fc13dd4157118d81cc99c53db7873540b0059 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 14:16:02 -0600 Subject: [PATCH 8/9] whitespace cleanup --- CIME/code_checker.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/CIME/code_checker.py b/CIME/code_checker.py index 6de5c14d34d..09707fd8258 100644 --- a/CIME/code_checker.py +++ b/CIME/code_checker.py @@ -24,7 +24,6 @@ logger = logging.getLogger(__name__) - ############################################################################### def _run_pylint(all_files, interactive): ############################################################################### @@ -81,16 +80,6 @@ def _run_pylint(all_files, interactive): return result - # if stat != 0: - # if interactive: - # logger.info("File %s has pylint problems, please fix\n Use command: %s" % (on_file, cmd)) - # logger.info(out + "\n" + err) - # return (on_file, out + "\n" + err) - # else: - # if interactive: - # logger.info("File %s has no pylint problems" % on_file) - # return (on_file, "") - ############################################################################### def _matches(file_path, file_ends): From b88c9e155912e6cfbb13e93f0b13621a89d823b8 Mon Sep 17 00:00:00 2001 From: Jim Edwards Date: Wed, 21 Aug 2024 14:30:38 -0600 Subject: [PATCH 9/9] try again with black --- CIME/code_checker.py | 1 + 1 file changed, 1 insertion(+) diff --git a/CIME/code_checker.py b/CIME/code_checker.py index 09707fd8258..ecd93cc3caf 100644 --- a/CIME/code_checker.py +++ b/CIME/code_checker.py @@ -24,6 +24,7 @@ logger = logging.getLogger(__name__) + ############################################################################### def _run_pylint(all_files, interactive): ###############################################################################