From 9f23721c2373643cad1e20a4df7e0613f899eee0 Mon Sep 17 00:00:00 2001 From: James Foucar Date: Tue, 11 Apr 2017 14:01:25 -0600 Subject: [PATCH] Refactor BUILD_THREADED to match documentation. Documentation (and CIME2 behavior) states that BUILD_THREADED is supposed to represent an override to force the model to be built with threads even if nthrds is one for all components. Until this PR, we had been using BUILD_THREADED as a marker to indicate that we found a component with nthrds > 1. This is not useful because we already had case.thread_count for that. This PR restores BUILD_THREADED to its original behavior and adds some tests to enforce. --- scripts/lib/CIME/SystemTests/erp.py | 1 - scripts/lib/CIME/build.py | 4 +-- scripts/lib/CIME/case.py | 13 +++++-- scripts/lib/CIME/case_setup.py | 6 ++-- scripts/lib/CIME/utils.py | 11 ------ scripts/tests/scripts_regression_tests.py | 41 ++++++++++++++++++++++- 6 files changed, 54 insertions(+), 22 deletions(-) diff --git a/scripts/lib/CIME/SystemTests/erp.py b/scripts/lib/CIME/SystemTests/erp.py index 2fbd4c87a61..9bade606e29 100644 --- a/scripts/lib/CIME/SystemTests/erp.py +++ b/scripts/lib/CIME/SystemTests/erp.py @@ -67,7 +67,6 @@ def build_phase(self, sharedlib_only=False, model_only=False): nthreads = self._case.get_value("NTHRDS_%s"%comp) rootpe = self._case.get_value("ROOTPE_%s"%comp) if ( nthreads > 1 ): - self._case.set_value("BUILD_THREADED", True) self._case.set_value("NTHRDS_%s"%comp, nthreads/2) if ( ntasks > 1 ): self._case.set_value("NTASKS_%s"%comp, ntasks/2) diff --git a/scripts/lib/CIME/build.py b/scripts/lib/CIME/build.py index 3c5a93ccf0e..74b34c2ef0f 100644 --- a/scripts/lib/CIME/build.py +++ b/scripts/lib/CIME/build.py @@ -335,7 +335,7 @@ def _clean_impl(case, cleanlist, clean_all): expect(cleanlist is not None and len(cleanlist) > 0,"Empty cleanlist not expected") debug = case.get_value("DEBUG") use_esmf_lib = case.get_value("USE_ESMF_LIB") - build_threaded = case.get_value("BUILD_THREADED") + build_threaded = case.get_build_threaded() gmake = case.get_value("GMAKE") caseroot = case.get_value("CASEROOT") casetools = case.get_value("CASETOOLS") @@ -394,7 +394,7 @@ def _case_build_impl(caseroot, case, sharedlib_only, model_only): # needs to be unset before building again. if "MODEL" in os.environ.keys(): del os.environ["MODEL"] - build_threaded = case.get_value("BUILD_THREADED") + build_threaded = case.get_build_threaded() casetools = case.get_value("CASETOOLS") exeroot = case.get_value("EXEROOT") incroot = case.get_value("INCROOT") diff --git a/scripts/lib/CIME/case.py b/scripts/lib/CIME/case.py index e02963c8205..09223c58dd8 100644 --- a/scripts/lib/CIME/case.py +++ b/scripts/lib/CIME/case.py @@ -10,7 +10,7 @@ from CIME.utils import expect, get_cime_root, append_status from CIME.utils import convert_to_type, get_model, get_project -from CIME.utils import get_build_threaded, get_current_commit +from CIME.utils import get_current_commit from CIME.check_lockedfiles import LOCKED_DIR, lock_file from CIME.XML.machines import Machines from CIME.XML.pes import Pes @@ -142,7 +142,7 @@ def _initialize_derived_attributes(self): mpi_attribs = { "compiler" : self.get_value("COMPILER"), "mpilib" : self.get_value("MPILIB"), - "threaded" : get_build_threaded(self) + "threaded" : self.get_build_threaded() } executable = env_mach_spec.get_mpirun(self, mpi_attribs, job="case.run", exe_only=True)[0] @@ -1072,7 +1072,7 @@ def get_mpirun_cmd(self, job="case.run"): mpi_attribs = { "compiler" : self.get_value("COMPILER"), "mpilib" : self.get_value("MPILIB"), - "threaded" : get_build_threaded(self) + "threaded" : self.get_build_threaded() } executable, args = env_mach_specific.get_mpirun(self, mpi_attribs, job=job) @@ -1119,6 +1119,13 @@ def load_env(self): env_module.load_env(compiler=compiler,debug=debug, mpilib=mpilib) self._is_env_loaded = True + def get_build_threaded(self): + """ + Returns True if current settings require a threaded build/run. + """ + force_threaded = self.get_value("BUILD_THREADED") + return bool(force_threaded) or self.thread_count > 1 + def _check_testlists(self, compset_alias, grid_name, files): """ CESM only: check the testlist file for tests of this compset grid combination diff --git a/scripts/lib/CIME/case_setup.py b/scripts/lib/CIME/case_setup.py index b6efaff3221..351a3437049 100644 --- a/scripts/lib/CIME/case_setup.py +++ b/scripts/lib/CIME/case_setup.py @@ -147,10 +147,8 @@ def _case_setup_impl(case, caseroot, clean=False, test_mode=False, reset=False, logger.debug("at update TOTALPES = %s"%pestot) case.set_value("TOTALPES", pestot) thread_count = env_mach_pes.get_max_thread_count(models) - if thread_count > 1: - case.set_value("BUILD_THREADED", True) - - expect(not (case.get_value("BUILD_THREADED") and compiler == "nag"), + build_threaded = case.get_build_threaded() + expect(not (build_threaded and compiler == "nag"), "it is not possible to run with OpenMP if using the NAG Fortran compiler") cost_pes = env_mach_pes.get_cost_pes(pestot, thread_count, machine=case.get_value("MACH")) case.set_value("COST_PES", cost_pes) diff --git a/scripts/lib/CIME/utils.py b/scripts/lib/CIME/utils.py index d0e362cbd28..8261d0587a8 100644 --- a/scripts/lib/CIME/utils.py +++ b/scripts/lib/CIME/utils.py @@ -973,17 +973,6 @@ def wait_for_unlocked(filepath): if file_object: file_object.close() -def get_build_threaded(case): - """Returns True if current settings require a threaded build/run.""" - force_threaded = case.get_value("BUILD_THREADED") - if force_threaded: - return True - comp_classes = case.get_values("COMP_CLASSES") - for comp_class in comp_classes: - if case.get_value("NTHRDS_%s"%comp_class) > 1: - return True - return False - def gunzip_existing_file(filepath): with gzip.open(filepath, "rb") as fd: return fd.read() diff --git a/scripts/tests/scripts_regression_tests.py b/scripts/tests/scripts_regression_tests.py index 3ef1dea3a14..892655f45af 100755 --- a/scripts/tests/scripts_regression_tests.py +++ b/scripts/tests/scripts_regression_tests.py @@ -1246,7 +1246,7 @@ class K_TestCimeCase(TestCreateTestCommon): ########################################################################### def test_cime_case(self): ########################################################################### - run_cmd_assert_result(self, "%s/create_test cime_test_only -t %s --no-build --test-root %s --output-root %s" + run_cmd_assert_result(self, "%s/create_test TESTRUNPASS_P1.f19_g16_rx1.A -t %s --no-build --test-root %s --output-root %s" % (SCRIPT_DIR, self._baseline_name, TEST_ROOT, TEST_ROOT)) casedir = os.path.join(self._testroot, @@ -1282,6 +1282,45 @@ def test_cime_case(self): # Test some test properties self.assertEqual(case.get_value("TESTCASE"), "TESTRUNPASS") + ########################################################################### + def test_cime_case_build_threaded_1(self): + ########################################################################### + run_cmd_assert_result(self, "%s/create_test TESTRUNPASS_P1x1.f19_g16_rx1.A -t %s --no-build --test-root %s --output-root %s" + % (SCRIPT_DIR, self._baseline_name, TEST_ROOT, TEST_ROOT)) + + casedir = os.path.join(self._testroot, + "%s.%s" % (CIME.utils.get_full_test_name("TESTRUNPASS_P1x1.f19_g16_rx1.A", machine=self._machine, compiler=self._compiler), self._baseline_name)) + self.assertTrue(os.path.isdir(casedir), msg="Missing casedir '%s'" % casedir) + + with Case(casedir, read_only=False) as case: + build_threaded = case.get_value("BUILD_THREADED") + self.assertFalse(build_threaded) + + build_threaded = case.get_build_threaded() + self.assertFalse(build_threaded) + + case.set_value("BUILD_THREADED", True) + + build_threaded = case.get_build_threaded() + self.assertTrue(build_threaded) + + ########################################################################### + def test_cime_case_build_threaded_2(self): + ########################################################################### + run_cmd_assert_result(self, "%s/create_test TESTRUNPASS_P1x2.f19_g16_rx1.A -t %s --no-build --test-root %s --output-root %s" + % (SCRIPT_DIR, self._baseline_name, TEST_ROOT, TEST_ROOT)) + + casedir = os.path.join(self._testroot, + "%s.%s" % (CIME.utils.get_full_test_name("TESTRUNPASS_P1x2.f19_g16_rx1.A", machine=self._machine, compiler=self._compiler), self._baseline_name)) + self.assertTrue(os.path.isdir(casedir), msg="Missing casedir '%s'" % casedir) + + with Case(casedir, read_only=False) as case: + build_threaded = case.get_value("BUILD_THREADED") + self.assertFalse(build_threaded) + + build_threaded = case.get_build_threaded() + self.assertTrue(build_threaded) + ########################################################################### def test_cime_case_mpi_serial(self): ###########################################################################