From ff3448b75a5f416cb1f3a59f5ab642c8524169c5 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 18 Nov 2024 09:07:01 -0700 Subject: [PATCH 01/26] run_neon: Print case path. --- python/ctsm/site_and_regional/tower_site.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/ctsm/site_and_regional/tower_site.py b/python/ctsm/site_and_regional/tower_site.py index 31c959cac7..137c932197 100644 --- a/python/ctsm/site_and_regional/tower_site.py +++ b/python/ctsm/site_and_regional/tower_site.py @@ -95,6 +95,7 @@ def build_base_case( if not output_root: output_root = os.getcwd() case_path = os.path.join(output_root, self.name) + print(case_path) logger.info("base_case_name : %s", self.name) logger.info("user_mods_dir : %s", user_mods_dirs[0]) From 535bec106272486d3a02a6b96cde2e19c6fd95ad Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 18 Nov 2024 09:08:11 -0700 Subject: [PATCH 02/26] run_neon: Fix bug where --overwrite is ignored. Resolves ESCOMP/CTSM#2884. --- python/ctsm/site_and_regional/neon_site.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index 70414d9e5c..ced4c4c64b 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -47,7 +47,8 @@ def build_base_case( user_mods_dirs = [ os.path.join(self.cesmroot, "cime_config", "usermods_dirs", "NEON", self.name) ] - case_path = super().build_base_case(cesmroot, output_root, res, compset, user_mods_dirs) + case_path = super().build_base_case(cesmroot, output_root, res, compset, user_mods_dirs, + overwrite=overwrite) return case_path From e74a63e3736678efc2aba4d47f6db36e1f2540c1 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 18 Nov 2024 09:11:11 -0700 Subject: [PATCH 03/26] run_neon: Improve help text for --base-case. --- python/ctsm/site_and_regional/neon_arg_parse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ctsm/site_and_regional/neon_arg_parse.py b/python/ctsm/site_and_regional/neon_arg_parse.py index 99f184dd62..db4a90c9e0 100644 --- a/python/ctsm/site_and_regional/neon_arg_parse.py +++ b/python/ctsm/site_and_regional/neon_arg_parse.py @@ -45,7 +45,7 @@ def get_parser(args, description, valid_neon_sites): "--base-case", help=""" Root Directory of base case build - [default: %(default)s] + [default: CESMROOT/NEONSITE] """, action="store", dest="base_case_root", From 49f44350b338790a255d605cfc511e7076729a4d Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 18 Nov 2024 09:29:41 -0700 Subject: [PATCH 04/26] run_neon: Fix issue where --setup-only was ignored. Resolves ESCOMP/CTSM#2885. --- python/ctsm/site_and_regional/neon_site.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index ced4c4c64b..073e0f75aa 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -48,7 +48,7 @@ def build_base_case( os.path.join(self.cesmroot, "cime_config", "usermods_dirs", "NEON", self.name) ] case_path = super().build_base_case(cesmroot, output_root, res, compset, user_mods_dirs, - overwrite=overwrite) + overwrite=overwrite, setup_only=setup_only) return case_path From 1caa2c955683ca202eb8c30b210f4b1222d7eaa9 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 18 Nov 2024 10:06:58 -0700 Subject: [PATCH 05/26] run_neon: Add --no-input-data-check option. --- python/ctsm/site_and_regional/neon_arg_parse.py | 17 +++++++++++++++++ python/ctsm/site_and_regional/neon_site.py | 4 ++++ python/ctsm/site_and_regional/run_neon.py | 2 ++ python/ctsm/site_and_regional/tower_site.py | 5 ++++- 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/python/ctsm/site_and_regional/neon_arg_parse.py b/python/ctsm/site_and_regional/neon_arg_parse.py index db4a90c9e0..3aa619aad3 100644 --- a/python/ctsm/site_and_regional/neon_arg_parse.py +++ b/python/ctsm/site_and_regional/neon_arg_parse.py @@ -91,6 +91,18 @@ def get_parser(args, description, valid_neon_sites): default=False, ) + parser.add_argument( + "--no-input-data-check", "--no-check-input-data", + help=""" + Don't check for input data. Implies --setup-only. + [default: %(default)s] + """, + action="store_true", + dest="no_input_data_check", + required=False, + default=False, + ) + parser.add_argument( "--rerun", help=""" @@ -223,6 +235,10 @@ def get_parser(args, description, valid_neon_sites): root_logger = logging.getLogger() root_logger.setLevel(logging.WARN) + # --no-input-data-check implies --setup-only + if args.no_input_data_check and not args.setup_only: + args.setup_only = True + return ( neon_sites, args.output_root, @@ -236,5 +252,6 @@ def get_parser(args, description, valid_neon_sites): args.setup_only, args.no_batch, args.rerun, + args.no_input_data_check, args.user_version, ) diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index 073e0f75aa..7978eac2ed 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -67,6 +67,7 @@ def run_case( no_batch=False, rerun=False, experiment=False, + no_input_data_check=False, ): """ Run case. @@ -93,6 +94,8 @@ def run_case( default False experiment: str, opt name of experiment, default False + no_input_data_check: bool, opt + default False """ user_mods_dirs = [ os.path.join(self.cesmroot, "cime_config", "usermods_dirs", "NEON", self.name) @@ -112,6 +115,7 @@ def run_case( no_batch, rerun, experiment, + no_input_data_check, ) def modify_user_nl(self, case_root, run_type, rundir, site_lines=None): diff --git a/python/ctsm/site_and_regional/run_neon.py b/python/ctsm/site_and_regional/run_neon.py index 3acbf435b1..d4ded645b7 100755 --- a/python/ctsm/site_and_regional/run_neon.py +++ b/python/ctsm/site_and_regional/run_neon.py @@ -194,6 +194,7 @@ def main(description): setup_only, no_batch, rerun, + no_input_data_check, user_version, ) = get_parser(sys.argv, description, valid_neon_sites) @@ -240,4 +241,5 @@ def main(description): no_batch=no_batch, rerun=rerun, experiment=experiment, + no_input_data_check=no_input_data_check, ) diff --git a/python/ctsm/site_and_regional/tower_site.py b/python/ctsm/site_and_regional/tower_site.py index 137c932197..7f83b29663 100644 --- a/python/ctsm/site_and_regional/tower_site.py +++ b/python/ctsm/site_and_regional/tower_site.py @@ -263,6 +263,7 @@ def run_case( no_batch, rerun, experiment, + no_input_data_check, ): """ Run case. @@ -413,8 +414,10 @@ def run_case( self.modify_user_nl(case_root, run_type, rundir) case.create_namelists() + # explicitly run check_input_data - case.check_all_input_data() + if not no_input_data_check: + case.check_all_input_data() if not setup_only: case.submit(no_batch=no_batch) print("-----------------------------------") From 3e63547b8b01db3f0ef41585f9af9f74ea23ee46 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Mon, 18 Nov 2024 10:25:19 -0700 Subject: [PATCH 06/26] run_neon: Add --xmlchange option. --- python/ctsm/site_and_regional/neon_arg_parse.py | 12 ++++++++++++ python/ctsm/site_and_regional/neon_site.py | 2 ++ python/ctsm/site_and_regional/run_neon.py | 2 ++ python/ctsm/site_and_regional/tower_site.py | 7 +++++++ 4 files changed, 23 insertions(+) diff --git a/python/ctsm/site_and_regional/neon_arg_parse.py b/python/ctsm/site_and_regional/neon_arg_parse.py index 3aa619aad3..7317c947fb 100644 --- a/python/ctsm/site_and_regional/neon_arg_parse.py +++ b/python/ctsm/site_and_regional/neon_arg_parse.py @@ -195,6 +195,17 @@ def get_parser(args, description, valid_neon_sites): choices=["v1", "v2", "v3"], ) + parser.add_argument( + "--xmlchange", + help=""" + Any xmlchanges (e.g., CLM_CO2_TYPE=constant,CCSM_CO2_PPMV=500) + [default: %(default)s] + """, + required=False, + type=str, + default=None, + ) + args = parse_args_and_handle_standard_logging_options(args, parser) if "all" in args.neon_sites: @@ -254,4 +265,5 @@ def get_parser(args, description, valid_neon_sites): args.rerun, args.no_input_data_check, args.user_version, + args.xmlchange, ) diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index 7978eac2ed..82ef7adf05 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -68,6 +68,7 @@ def run_case( rerun=False, experiment=False, no_input_data_check=False, + xmlchange=None, ): """ Run case. @@ -116,6 +117,7 @@ def run_case( rerun, experiment, no_input_data_check, + xmlchange, ) def modify_user_nl(self, case_root, run_type, rundir, site_lines=None): diff --git a/python/ctsm/site_and_regional/run_neon.py b/python/ctsm/site_and_regional/run_neon.py index d4ded645b7..2d9346d49e 100755 --- a/python/ctsm/site_and_regional/run_neon.py +++ b/python/ctsm/site_and_regional/run_neon.py @@ -196,6 +196,7 @@ def main(description): rerun, no_input_data_check, user_version, + xmlchange, ) = get_parser(sys.argv, description, valid_neon_sites) if output_root: @@ -242,4 +243,5 @@ def main(description): rerun=rerun, experiment=experiment, no_input_data_check=no_input_data_check, + xmlchange=xmlchange, ) diff --git a/python/ctsm/site_and_regional/tower_site.py b/python/ctsm/site_and_regional/tower_site.py index 7f83b29663..33b36b7613 100644 --- a/python/ctsm/site_and_regional/tower_site.py +++ b/python/ctsm/site_and_regional/tower_site.py @@ -264,6 +264,7 @@ def run_case( rerun, experiment, no_input_data_check, + xmlchange, ): """ Run case. @@ -411,6 +412,12 @@ def run_case( if not rundir: rundir = case.get_value("RUNDIR") + if xmlchange: + xmlchange_list = xmlchange.split(",") + for setting in xmlchange_list: + setting_split = setting.split("=") + case.set_value(*setting_split) + self.modify_user_nl(case_root, run_type, rundir) case.create_namelists() From daebec2971dfdf5b6d65379b1d8accf8e9d747f7 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Wed, 8 Jan 2025 10:39:39 -0700 Subject: [PATCH 07/26] Remove "leaving fates model" message. --- src/utils/clmfates_interfaceMod.F90 | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/utils/clmfates_interfaceMod.F90 b/src/utils/clmfates_interfaceMod.F90 index 269189d1b7..9b2ac6d6c4 100644 --- a/src/utils/clmfates_interfaceMod.F90 +++ b/src/utils/clmfates_interfaceMod.F90 @@ -1279,11 +1279,6 @@ subroutine dynamics_driv(this, nc, bounds_clump, & this%fates(nc)%sites, & this%fates(nc)%bc_in ) - if (masterproc) then - write(iulog, *) 'clm: leaving fates model', bounds_clump%begg, & - bounds_clump%endg - end if - call t_stopf('fates_dynamics_daily_driver') return From b816f11fa36c5b88ac5898bc3ae65d064b015361 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Wed, 8 Jan 2025 10:41:06 -0700 Subject: [PATCH 08/26] Remove "CN Soil matrix solution is off" message. --- src/soilbiogeochem/CNSoilMatrixMod.F90 | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/soilbiogeochem/CNSoilMatrixMod.F90 b/src/soilbiogeochem/CNSoilMatrixMod.F90 index 56c1f11b5c..be3ab866a2 100644 --- a/src/soilbiogeochem/CNSoilMatrixMod.F90 +++ b/src/soilbiogeochem/CNSoilMatrixMod.F90 @@ -79,8 +79,6 @@ subroutine CNSoilMatrixInit( ) else write(iulog,*) ' no extra matrix solution tracability output' end if - else - write(iulog,*) 'CN Soil matrix solution is off' end if end subroutine CNSoilMatrixInit From d9fb22de59555ffee1485a830967168f884c145f Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Wed, 8 Jan 2025 10:42:29 -0700 Subject: [PATCH 09/26] "CN Soil matrix solution is on" msg now only on masterproc. --- src/soilbiogeochem/CNSoilMatrixMod.F90 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/soilbiogeochem/CNSoilMatrixMod.F90 b/src/soilbiogeochem/CNSoilMatrixMod.F90 index be3ab866a2..e2b4ff2292 100644 --- a/src/soilbiogeochem/CNSoilMatrixMod.F90 +++ b/src/soilbiogeochem/CNSoilMatrixMod.F90 @@ -18,6 +18,7 @@ module CNSoilMatrixMod use shr_log_mod , only : errMsg => shr_log_errMsg use decompMod , only : bounds_type use abortutils , only : endrun + use spmdMod , only : masterproc use clm_time_manager , only : get_step_size, is_end_curr_month,get_curr_date,update_DA_nstep use clm_time_manager , only : is_first_restart_step,is_beg_curr_year,is_end_curr_year,is_first_step_of_this_run_segment use clm_varpar , only : ndecomp_pools, nlevdecomp, ndecomp_pools_vr !number of biogeochemically active soil layers @@ -64,7 +65,7 @@ subroutine CNSoilMatrixInit( ) ! !LOCAL VARIABLES: !----------------------------------------------------------------------- - if ( use_soil_matrixcn ) then + if ( use_soil_matrixcn .and. masterproc) then write(iulog,*) 'CN Soil matrix solution is on' write(iulog,*) '*****************************' if ( spinup_matrixcn ) then From a24c8f06ce52f819ccaa6631ce74c310ed624e34 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Wed, 8 Jan 2025 10:42:59 -0700 Subject: [PATCH 10/26] Remove "calling FireInit" message. --- src/utils/clmfates_interfaceMod.F90 | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/clmfates_interfaceMod.F90 b/src/utils/clmfates_interfaceMod.F90 index 9b2ac6d6c4..ed66eb91ce 100644 --- a/src/utils/clmfates_interfaceMod.F90 +++ b/src/utils/clmfates_interfaceMod.F90 @@ -3133,7 +3133,6 @@ subroutine Init2(this, bounds, NLFilename) call t_startf('fates_init2') - write(iulog,*) 'Init2: calling FireInit' call this%fates_fire_data_method%FireInit(bounds, NLFilename) call t_stopf('fates_init2') From 871cfa950eaf0d1c27c0db2db03d457f55319b9c Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Wed, 8 Jan 2025 11:58:21 -0700 Subject: [PATCH 11/26] Restrict "for_testing_allow_interp_non_ciso_to_ciso is .true." msg to masterproc. --- src/init_interp/initInterp.F90 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/init_interp/initInterp.F90 b/src/init_interp/initInterp.F90 index f6027fe632..e0d56aed62 100644 --- a/src/init_interp/initInterp.F90 +++ b/src/init_interp/initInterp.F90 @@ -1397,7 +1397,7 @@ subroutine check_interp_non_ciso_to_ciso(ncidi) write(iulog,*) 'Proceeding despite missing c13 and/or c14 fields on input finidat file,' write(iulog,*) 'because for_testing_allow_interp_non_ciso_to_ciso is set.' write(iulog,*) ' ' - else + else if (masterproc) then write(iulog,*) ' ' write(iulog,*) 'for_testing_allow_interp_non_ciso_to_ciso is .true., but it appears to be unnecessary in this run' write(iulog,*) '(this is informational only - it does not indicate a problem)' From e386a21965f753b0b65c230ff111d440420384c5 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 13 Feb 2025 13:46:25 -0700 Subject: [PATCH 12/26] Use no_input_data_check and xmlchange in Plumber2Site. --- python/ctsm/site_and_regional/plumber_site.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/python/ctsm/site_and_regional/plumber_site.py b/python/ctsm/site_and_regional/plumber_site.py index 3f06f2949b..7d03af8a40 100755 --- a/python/ctsm/site_and_regional/plumber_site.py +++ b/python/ctsm/site_and_regional/plumber_site.py @@ -49,7 +49,8 @@ def build_base_case( self.cesmroot, "cime_config", "usermods_dirs", "clm", "PLUMBER2", self.name ) ] - case_path = super().build_base_case(cesmroot, output_root, res, compset, user_mods_dirs) + case_path = super().build_base_case(cesmroot, output_root, res, compset, user_mods_dirs, + overwrite=overwrite, setup_only=setup_only) return case_path @@ -67,6 +68,8 @@ def run_case( no_batch=False, rerun=False, experiment=False, + no_input_data_check=False, + xmlchange=None, ): """ Run case. @@ -113,6 +116,8 @@ def run_case( no_batch, rerun, experiment, + no_input_data_check, + xmlchange, ) def set_ref_case(self, case): From e8fc526e0d7818d45f171488c78392c4ff63902a Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 13 Feb 2025 13:58:06 -0700 Subject: [PATCH 13/26] Reformat with black. --- python/ctsm/site_and_regional/neon_site.py | 11 +++++++++-- python/ctsm/site_and_regional/plumber_site.py | 11 +++++++++-- python/ctsm/site_and_regional/tower_arg_parse.py | 3 ++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index e6f905ce82..3da43ac27a 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -50,8 +50,15 @@ def build_base_case( self.cesmroot, "cime_config", "usermods_dirs", "clm", "NEON", self.name ) ] - case_path = super().build_base_case(cesmroot, output_root, res, compset, user_mods_dirs, - overwrite=overwrite, setup_only=setup_only) + case_path = super().build_base_case( + cesmroot, + output_root, + res, + compset, + user_mods_dirs, + overwrite=overwrite, + setup_only=setup_only, + ) return case_path diff --git a/python/ctsm/site_and_regional/plumber_site.py b/python/ctsm/site_and_regional/plumber_site.py index 7d03af8a40..4048312627 100755 --- a/python/ctsm/site_and_regional/plumber_site.py +++ b/python/ctsm/site_and_regional/plumber_site.py @@ -49,8 +49,15 @@ def build_base_case( self.cesmroot, "cime_config", "usermods_dirs", "clm", "PLUMBER2", self.name ) ] - case_path = super().build_base_case(cesmroot, output_root, res, compset, user_mods_dirs, - overwrite=overwrite, setup_only=setup_only) + case_path = super().build_base_case( + cesmroot, + output_root, + res, + compset, + user_mods_dirs, + overwrite=overwrite, + setup_only=setup_only, + ) return case_path diff --git a/python/ctsm/site_and_regional/tower_arg_parse.py b/python/ctsm/site_and_regional/tower_arg_parse.py index 21dfef7836..29d04b710a 100644 --- a/python/ctsm/site_and_regional/tower_arg_parse.py +++ b/python/ctsm/site_and_regional/tower_arg_parse.py @@ -105,7 +105,8 @@ def get_parser(args, description, valid_neon_sites, valid_plumber_sites): ) parser.add_argument( - "--no-input-data-check", "--no-check-input-data", + "--no-input-data-check", + "--no-check-input-data", help=""" Don't check for input data. Implies --setup-only. [default: %(default)s] From 7cecdf3ad112e6d059a7554a0c765854bc986c19 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 13 Feb 2025 13:58:47 -0700 Subject: [PATCH 14/26] Add previous commit to .git-blame-ignore-revs. --- .git-blame-ignore-revs | 1 + 1 file changed, 1 insertion(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index 8960800172..1e73e65b4f 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -58,3 +58,4 @@ bd535c710db78420b8e8b9d71d88d8339e899c59 4b20bbd7003e6f77dab4e3268cc4a43f9b5a3b5d cf433215b58ba8776ec5edfb0b0d80c0836ed3a0 16d57ff37859b34dab005693e3085d64e2bcd95a +e8fc526e0d7818d45f171488c78392c4ff63902a From f0d56c4630b617501178848db5f71cdf4579289f Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 13 Feb 2025 14:00:43 -0700 Subject: [PATCH 15/26] Use --no-input-data-check in run_tower testing. --- python/ctsm/test/test_sys_run_tower.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/ctsm/test/test_sys_run_tower.py b/python/ctsm/test/test_sys_run_tower.py index b1eef6c3a9..6141e562b9 100755 --- a/python/ctsm/test/test_sys_run_tower.py +++ b/python/ctsm/test/test_sys_run_tower.py @@ -58,7 +58,7 @@ def test_one_site(self): os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), "--neon-sites", "BART", - "--setup-only", + "--no-input-data-check", "--experiment", "TEST", "--output-root", @@ -82,7 +82,7 @@ def test_ad_site(self): os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), "--neon-sites", "ABBY", - "--setup-only", + "--no-input-data-check", "--run-type", "ad", "--output-root", @@ -104,7 +104,7 @@ def test_plumber_site(self): os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), "--plumber-sites", "AR-SLu", - "--setup-only", + "--no-input-data-check", "--experiment", "TEST", "--output-root", From d7c058bbd1c5c4e4496ced80eb9ab0d5c970dd5b Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 13 Feb 2025 14:48:28 -0700 Subject: [PATCH 16/26] Add run_tower system tests of --xmlchange. --- python/ctsm/test/test_sys_run_tower.py | 65 ++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/python/ctsm/test/test_sys_run_tower.py b/python/ctsm/test/test_sys_run_tower.py index 6141e562b9..3520adabe5 100755 --- a/python/ctsm/test/test_sys_run_tower.py +++ b/python/ctsm/test/test_sys_run_tower.py @@ -15,6 +15,8 @@ from ctsm.site_and_regional.run_tower import main from ctsm.path_utils import path_to_ctsm_root +from CIME.case import Case + # Allow test names that pylint doesn't like; otherwise hard to make them # readable # pylint: disable=invalid-name @@ -115,6 +117,69 @@ def test_plumber_site(self): # assert that AR-SLu directories were created during setup self.assertTrue("AR-SLu" in glob.glob(self._tempdir + "/AR-SLu*")[0]) + def test_xmlchange_neon(self): + """ + This test checks that the --xmlchange argument is obeyed for a NEON site. + """ + + # run the run_tower tool + sys.argv = [ + os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), + "--neon-sites", + "BART", + "--no-input-data-check", + "--experiment", + "TEST", + "--xmlchange", + "CLM_CO2_TYPE=constant,CCSM_CO2_PPMV=1987", + "--output-root", + self._tempdir, + ] + print(sys.argv) + main("") + + # Check that the --xmlchange argument is obeyed + case_dir = os.path.join(self._tempdir, "BART.TEST.transient") + self.assertTrue(os.path.exists(case_dir)) + with Case(case_dir, read_only=True) as case: + value = case.get_value("CLM_CO2_TYPE") + print(f"CLM_CO2_TYPE = {value}") + self.assertTrue(value == "constant") + value = int(case.get_value("CCSM_CO2_PPMV")) + print(f"CCSM_CO2_PPMV = {value}") + self.assertTrue(int(value) == 1987) + + def test_xmlchange_plumber(self): + """ + This test checks that the --xmlchange argument is obeyed for a PLUMBER site. + """ + + # run the run_tower tool for plumber site + sys.argv = [ + os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), + "--plumber-sites", + "AR-SLu", + "--no-input-data-check", + "--experiment", + "TEST", + "--xmlchange", + "CLM_CO2_TYPE=constant,CCSM_CO2_PPMV=1987", + "--output-root", + self._tempdir, + ] + main("") + + # Check that the --xmlchange argument is obeyed + case_dir = os.path.join(self._tempdir, "AR-SLu.TEST.ad") + self.assertTrue(os.path.exists(case_dir)) + with Case(case_dir, read_only=True) as case: + value = case.get_value("CLM_CO2_TYPE") + print(f"CLM_CO2_TYPE = {value}") + self.assertTrue(value == "constant") + value = int(case.get_value("CCSM_CO2_PPMV")) + print(f"CCSM_CO2_PPMV = {value}") + self.assertTrue(int(value) == 1987) + if __name__ == "__main__": unit_testing.setup_for_tests() From 1a9535102eb9e057d33865255defebc1d2090113 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 13 Feb 2025 21:09:40 -0700 Subject: [PATCH 17/26] Add run_tower system tests of --setup-only. --- python/ctsm/test/test_sys_run_tower.py | 51 ++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/python/ctsm/test/test_sys_run_tower.py b/python/ctsm/test/test_sys_run_tower.py index 3520adabe5..b6e414319d 100755 --- a/python/ctsm/test/test_sys_run_tower.py +++ b/python/ctsm/test/test_sys_run_tower.py @@ -180,6 +180,57 @@ def test_xmlchange_plumber(self): print(f"CCSM_CO2_PPMV = {value}") self.assertTrue(int(value) == 1987) + def test_setup_only_neon(self): + """ + This test checks that the --setup-only argument is obeyed for NEON sites + """ + + # run the run_tower tool + site_name = "BART" + sys.argv = [ + os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), + "--neon-sites", + site_name, + "--setup-only", + "--experiment", + "TEST", + "--output-root", + self._tempdir, + ] + print(sys.argv) + main("") + + # make sure that build didn't happen: this dir should be empty + case_dir = os.path.join(self._tempdir, site_name) + build_dir_to_check = os.path.join(case_dir, "bld", "cpl", "obj") + self.assertTrue(os.path.exists(build_dir_to_check)) + self.assertTrue(len(os.listdir(build_dir_to_check)) == 0) + + def test_setup_only_plumber(self): + """ + This test checks that the --setup-only argument is obeyed for PLUMBER sites + """ + + # run the run_tower tool for plumber site + site_name = "AR-SLu" + sys.argv = [ + os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), + "--plumber-sites", + site_name, + "--setup-only", + "--experiment", + "TEST", + "--output-root", + self._tempdir, + ] + main("") + + # make sure that build didn't happen: this dir should be empty + case_dir = os.path.join(self._tempdir, site_name) + build_dir_to_check = os.path.join(case_dir, "bld", "cpl", "obj") + self.assertTrue(os.path.exists(build_dir_to_check)) + self.assertTrue(len(os.listdir(build_dir_to_check)) == 0) + if __name__ == "__main__": unit_testing.setup_for_tests() From 6fd238f19be95f702ac47a229ca04c897a245360 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Thu, 13 Feb 2025 21:46:21 -0700 Subject: [PATCH 18/26] Add run_tower system tests of --overwrite. --- python/ctsm/test/test_sys_run_tower.py | 68 ++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/python/ctsm/test/test_sys_run_tower.py b/python/ctsm/test/test_sys_run_tower.py index b6e414319d..0639563229 100755 --- a/python/ctsm/test/test_sys_run_tower.py +++ b/python/ctsm/test/test_sys_run_tower.py @@ -10,6 +10,7 @@ import tempfile import shutil import sys +import pathlib from ctsm import unit_testing from ctsm.site_and_regional.run_tower import main @@ -231,6 +232,73 @@ def test_setup_only_plumber(self): self.assertTrue(os.path.exists(build_dir_to_check)) self.assertTrue(len(os.listdir(build_dir_to_check)) == 0) + def test_overwrite_neon(self): + """ + This test checks that the --overwrite argument is obeyed for NEON sites + """ + + # run the run_tower tool once + site_name = "BART" + sys.argv = [ + os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), + "--neon-sites", + site_name, + "--no-input-data-check", + "--experiment", + "TEST", + "--output-root", + self._tempdir, + ] + print(sys.argv) + main("") + + # create a file that should be erased during the upcoming overwrite + case_dir = os.path.join(self._tempdir, site_name) + test_file = os.path.join(case_dir, "test_file") + pathlib.Path(test_file).touch() + self.assertTrue(os.path.exists(test_file)) + + # run the tool again, overwriting existing + sys.argv += ["--overwrite"] + print(sys.argv) + main("") + + # ensure that file we created is gone + self.assertFalse(os.path.exists(test_file)) + + def test_overwrite_plumber(self): + """ + This test checks that the --overwrite argument is obeyed for PLUMBER sites + """ + + # run the run_tower tool once + site_name = "AR-SLu" + sys.argv = [ + os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), + "--plumber-sites", + site_name, + "--no-input-data-check", + "--experiment", + "TEST", + "--output-root", + self._tempdir, + ] + main("") + + # create a file that should be erased during the upcoming overwrite + case_dir = os.path.join(self._tempdir, site_name) + test_file = os.path.join(case_dir, "test_file") + pathlib.Path(test_file).touch() + self.assertTrue(os.path.exists(test_file)) + + # run the tool again, overwriting existing + sys.argv += ["--overwrite"] + print(sys.argv) + main("") + + # ensure that file we created is gone + self.assertFalse(os.path.exists(test_file)) + if __name__ == "__main__": unit_testing.setup_for_tests() From bf4e6cf828759bb51a072dcee86fdc5b431632b9 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Fri, 14 Feb 2025 09:14:23 -0700 Subject: [PATCH 19/26] Satisfy pylint. --- python/ctsm/test/test_sys_run_tower.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/ctsm/test/test_sys_run_tower.py b/python/ctsm/test/test_sys_run_tower.py index 0639563229..19c3d5701a 100755 --- a/python/ctsm/test/test_sys_run_tower.py +++ b/python/ctsm/test/test_sys_run_tower.py @@ -16,6 +16,8 @@ from ctsm.site_and_regional.run_tower import main from ctsm.path_utils import path_to_ctsm_root +# pylint: disable=import-error +# pylint: disable=wrong-import-order from CIME.case import Case # Allow test names that pylint doesn't like; otherwise hard to make them From 10c92700f460438c46f541755930c44a1f350e60 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Sat, 15 Feb 2025 10:29:35 -0700 Subject: [PATCH 20/26] Remove build_base_case method from tower child types. --- python/ctsm/site_and_regional/neon_site.py | 35 +----- python/ctsm/site_and_regional/plumber_site.py | 33 +----- python/ctsm/site_and_regional/run_tower.py | 6 +- python/ctsm/site_and_regional/tower_site.py | 21 +++- python/ctsm/test/test_sys_run_tower.py | 101 ++---------------- python/ctsm/test/test_unit_neon_site.py | 12 +-- python/ctsm/test/test_unit_tower_site.py | 70 ++++++++++++ 7 files changed, 107 insertions(+), 171 deletions(-) create mode 100755 python/ctsm/test/test_unit_tower_site.py diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index 3da43ac27a..f40ee974bf 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -34,34 +34,6 @@ class NeonSite(TowerSite): A class for encapsulating neon sites. """ - def build_base_case( - self, - cesmroot, - output_root, - res, - compset, - user_mods_dirs=None, - overwrite=False, - setup_only=False, - ): - if user_mods_dirs is None: - user_mods_dirs = [ - os.path.join( - self.cesmroot, "cime_config", "usermods_dirs", "clm", "NEON", self.name - ) - ] - case_path = super().build_base_case( - cesmroot, - output_root, - res, - compset, - user_mods_dirs, - overwrite=overwrite, - setup_only=setup_only, - ) - - return case_path - # pylint: disable=too-many-statements def run_case( self, @@ -69,7 +41,6 @@ def run_case( run_type, prism, user_version, - tower_type=None, user_mods_dirs=None, overwrite=False, setup_only=False, @@ -106,16 +77,16 @@ def run_case( default False """ user_mods_dirs = [ - os.path.join(self.cesmroot, "cime_config", "usermods_dirs", "clm", "NEON", self.name) + os.path.join( + self.cesmroot, "cime_config", "usermods_dirs", "clm", self.tower_type, self.name + ) ] - tower_type = "NEON" super().run_case( base_case_root, run_type, prism, user_version, - tower_type, user_mods_dirs, overwrite, setup_only, diff --git a/python/ctsm/site_and_regional/plumber_site.py b/python/ctsm/site_and_regional/plumber_site.py index 4048312627..c8d7f2fc59 100755 --- a/python/ctsm/site_and_regional/plumber_site.py +++ b/python/ctsm/site_and_regional/plumber_site.py @@ -33,34 +33,6 @@ class Plumber2Site(TowerSite): A class for encapsulating plumber sites. """ - def build_base_case( - self, - cesmroot, - output_root, - res, - compset, - user_mods_dirs=None, - overwrite=False, - setup_only=False, - ): - if user_mods_dirs is None: - user_mods_dirs = [ - os.path.join( - self.cesmroot, "cime_config", "usermods_dirs", "clm", "PLUMBER2", self.name - ) - ] - case_path = super().build_base_case( - cesmroot, - output_root, - res, - compset, - user_mods_dirs, - overwrite=overwrite, - setup_only=setup_only, - ) - - return case_path - # pylint: disable=too-many-statements def run_case( self, @@ -68,7 +40,6 @@ def run_case( run_type, prism, user_version, - tower_type=None, user_mods_dirs=None, overwrite=False, setup_only=False, @@ -107,16 +78,14 @@ def run_case( """ user_mods_dirs = [ os.path.join( - self.cesmroot, "cime_config", "usermods_dirs", "clm", "PLUMBER2", self.name + self.cesmroot, "cime_config", "usermods_dirs", "clm", self.tower_type, self.name ) ] - tower_type = "PLUMBER" super().run_case( base_case_root, run_type, prism, user_version, - tower_type, user_mods_dirs, overwrite, setup_only, diff --git a/python/ctsm/site_and_regional/run_tower.py b/python/ctsm/site_and_regional/run_tower.py index aecda90742..5a1b50c6d1 100755 --- a/python/ctsm/site_and_regional/run_tower.py +++ b/python/ctsm/site_and_regional/run_tower.py @@ -160,7 +160,9 @@ def parse_neon_listing(listing_file, valid_neon_sites): if site_name in line: finidat = line.split(",")[0].split("/")[-1] - neon_site = NeonSite(site_name, start_year, end_year, start_month, end_month, finidat) + neon_site = NeonSite( + "NEON", site_name, start_year, end_year, start_month, end_month, finidat + ) logger.debug(neon_site) available_list.append(neon_site) @@ -192,7 +194,7 @@ def setup_plumber_data(valid_plumber_sites): finidat = None plumber_site = Plumber2Site( - site_name, start_year, end_year, start_month, end_month, finidat + "PLUMBER2", site_name, start_year, end_year, start_month, end_month, finidat ) available_list.append(plumber_site) diff --git a/python/ctsm/site_and_regional/tower_site.py b/python/ctsm/site_and_regional/tower_site.py index f03e56d819..ccff184fd7 100644 --- a/python/ctsm/site_and_regional/tower_site.py +++ b/python/ctsm/site_and_regional/tower_site.py @@ -29,6 +29,7 @@ logger = logging.getLogger(__name__) +ALLOWED_SITE_TYPES = ["NEON", "PLUMBER2"] # pylint: disable=too-many-instance-attributes class TowerSite: @@ -41,12 +42,18 @@ class TowerSite: ------- """ - def __init__(self, name, start_year, end_year, start_month, end_month, finidat): + def __init__(self, tower_type, name, start_year, end_year, start_month, end_month, finidat): """ Initializes TowerSite with the given arguments. Parameters ---------- """ + if tower_type not in ALLOWED_SITE_TYPES: + raise ValueError( + f"tower_type '{tower_type}' not allowed. " + f"Choose from: {','.join(ALLOWED_SITE_TYPES)}" + ) + self.tower_type = tower_type self.name = name self.start_year = int(start_year) self.end_year = int(end_year) @@ -98,6 +105,13 @@ def build_base_case( setup_only (bool) : Flag to only do the setup phase """ + # Define fallback user_mods_dirs + if user_mods_dirs is None: + user_mods_dirs = [ + os.path.join( + self.cesmroot, "cime_config", "usermods_dirs", "clm", self.tower_type, self.name + ) + ] # # Error checking on the input # @@ -111,7 +125,7 @@ def build_base_case( abort("Input compset is NOT a boolean as expected: " + str(compset)) if not isinstance(setup_only, bool): abort("Input setup_only is NOT a boolean as expected: " + str(setup_only)) - if not isinstance(user_mods_dirs, list): + if user_mods_dirs is not None and not isinstance(user_mods_dirs, list): abort("Input user_mods_dirs is NOT a list as expected: " + str(user_mods_dirs)) for dirtree in user_mods_dirs: if not os.path.isdir(dirtree): @@ -290,7 +304,6 @@ def run_case( run_type, prism, user_version, - tower_type, user_mods_dirs, overwrite, setup_only, @@ -408,7 +421,7 @@ def run_case( case.set_value("STOP_OPTION", "ndays") case.set_value("REST_OPTION", "end") case.set_value("CONTINUE_RUN", False) - if tower_type == "NEON": + if self.tower_type == "NEON": case.set_value("NEONVERSION", version) if prism: case.set_value("CLM_USRDAT_NAME", "NEON.PRISM") diff --git a/python/ctsm/test/test_sys_run_tower.py b/python/ctsm/test/test_sys_run_tower.py index 19c3d5701a..de69101513 100755 --- a/python/ctsm/test/test_sys_run_tower.py +++ b/python/ctsm/test/test_sys_run_tower.py @@ -120,9 +120,9 @@ def test_plumber_site(self): # assert that AR-SLu directories were created during setup self.assertTrue("AR-SLu" in glob.glob(self._tempdir + "/AR-SLu*")[0]) - def test_xmlchange_neon(self): + def test_xmlchange(self): """ - This test checks that the --xmlchange argument is obeyed for a NEON site. + This test checks that the --xmlchange argument is obeyed. """ # run the run_tower tool @@ -152,40 +152,9 @@ def test_xmlchange_neon(self): print(f"CCSM_CO2_PPMV = {value}") self.assertTrue(int(value) == 1987) - def test_xmlchange_plumber(self): + def test_setup_only(self): """ - This test checks that the --xmlchange argument is obeyed for a PLUMBER site. - """ - - # run the run_tower tool for plumber site - sys.argv = [ - os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), - "--plumber-sites", - "AR-SLu", - "--no-input-data-check", - "--experiment", - "TEST", - "--xmlchange", - "CLM_CO2_TYPE=constant,CCSM_CO2_PPMV=1987", - "--output-root", - self._tempdir, - ] - main("") - - # Check that the --xmlchange argument is obeyed - case_dir = os.path.join(self._tempdir, "AR-SLu.TEST.ad") - self.assertTrue(os.path.exists(case_dir)) - with Case(case_dir, read_only=True) as case: - value = case.get_value("CLM_CO2_TYPE") - print(f"CLM_CO2_TYPE = {value}") - self.assertTrue(value == "constant") - value = int(case.get_value("CCSM_CO2_PPMV")) - print(f"CCSM_CO2_PPMV = {value}") - self.assertTrue(int(value) == 1987) - - def test_setup_only_neon(self): - """ - This test checks that the --setup-only argument is obeyed for NEON sites + This test checks that the --setup-only argument is obeyed """ # run the run_tower tool @@ -209,34 +178,9 @@ def test_setup_only_neon(self): self.assertTrue(os.path.exists(build_dir_to_check)) self.assertTrue(len(os.listdir(build_dir_to_check)) == 0) - def test_setup_only_plumber(self): - """ - This test checks that the --setup-only argument is obeyed for PLUMBER sites - """ - - # run the run_tower tool for plumber site - site_name = "AR-SLu" - sys.argv = [ - os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), - "--plumber-sites", - site_name, - "--setup-only", - "--experiment", - "TEST", - "--output-root", - self._tempdir, - ] - main("") - - # make sure that build didn't happen: this dir should be empty - case_dir = os.path.join(self._tempdir, site_name) - build_dir_to_check = os.path.join(case_dir, "bld", "cpl", "obj") - self.assertTrue(os.path.exists(build_dir_to_check)) - self.assertTrue(len(os.listdir(build_dir_to_check)) == 0) - - def test_overwrite_neon(self): + def test_overwrite(self): """ - This test checks that the --overwrite argument is obeyed for NEON sites + This test checks that the --overwrite argument is obeyed """ # run the run_tower tool once @@ -268,39 +212,6 @@ def test_overwrite_neon(self): # ensure that file we created is gone self.assertFalse(os.path.exists(test_file)) - def test_overwrite_plumber(self): - """ - This test checks that the --overwrite argument is obeyed for PLUMBER sites - """ - - # run the run_tower tool once - site_name = "AR-SLu" - sys.argv = [ - os.path.join(path_to_ctsm_root(), "tools", "site_and_regional", "run_tower"), - "--plumber-sites", - site_name, - "--no-input-data-check", - "--experiment", - "TEST", - "--output-root", - self._tempdir, - ] - main("") - - # create a file that should be erased during the upcoming overwrite - case_dir = os.path.join(self._tempdir, site_name) - test_file = os.path.join(case_dir, "test_file") - pathlib.Path(test_file).touch() - self.assertTrue(os.path.exists(test_file)) - - # run the tool again, overwriting existing - sys.argv += ["--overwrite"] - print(sys.argv) - main("") - - # ensure that file we created is gone - self.assertFalse(os.path.exists(test_file)) - if __name__ == "__main__": unit_testing.setup_for_tests() diff --git a/python/ctsm/test/test_unit_neon_site.py b/python/ctsm/test/test_unit_neon_site.py index 909bd3e3c0..f578f02b82 100755 --- a/python/ctsm/test/test_unit_neon_site.py +++ b/python/ctsm/test/test_unit_neon_site.py @@ -62,9 +62,9 @@ def test_modify_user_nl_transient(self): rundir = "" # create NeonSite object and update namelist - NeonSite(name, start_year, end_year, start_month, end_month, finidat).modify_user_nl( - case_root, run_type, rundir - ) + NeonSite( + "NEON", name, start_year, end_year, start_month, end_month, finidat + ).modify_user_nl(case_root, run_type, rundir) # gather file contents for test new_nl_file = open(glob.glob(case_root + "/*")[0], "r") @@ -97,9 +97,9 @@ def test_modify_user_nl_ad(self): rundir = "" # create NeonSite object and update namelist - NeonSite(name, start_year, end_year, start_month, end_month, finidat).modify_user_nl( - case_root, run_type, rundir - ) + NeonSite( + "NEON", name, start_year, end_year, start_month, end_month, finidat + ).modify_user_nl(case_root, run_type, rundir) # gather file contents for test new_nl_file = open(glob.glob(case_root + "/*")[0], "r") diff --git a/python/ctsm/test/test_unit_tower_site.py b/python/ctsm/test/test_unit_tower_site.py new file mode 100755 index 0000000000..5310e2eda1 --- /dev/null +++ b/python/ctsm/test/test_unit_tower_site.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +""" +Unit tests for tower_site that aren't specific to a single tower type + +You can run this by: + python -m unittest test_unit_tower_site.py +""" + +import unittest +import tempfile +import shutil +import os +import sys + +# -- add python/ctsm to path (needed if we want to run the test stand-alone) +_CTSM_PYTHON = os.path.join(os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir) +sys.path.insert(1, _CTSM_PYTHON) + +# pylint: disable=wrong-import-position +from ctsm import unit_testing +from ctsm.site_and_regional.tower_site import TowerSite, ALLOWED_SITE_TYPES + +# pylint: disable=invalid-name + + +class Test_tower_site(unittest.TestCase): + """ + Basic class for testing tower_site.py for things not specific to any tower type. + """ + + def setUp(self): + """ + Make /_tempdir for use by these tests. + """ + self._previous_dir = os.getcwd() + self._tempdir = tempfile.mkdtemp() + + def tearDown(self): + """ + Remove temporary directory + """ + os.chdir(self._previous_dir) + shutil.rmtree(self._tempdir, ignore_errors=True) + + def test_disallowed_tower_types(self): + """ + Test that an invalid tower type throws an error + """ + # NeonSite parameters (not that it matters) + name = "ABBY" + start_year = 2020 + end_year = 2021 + start_month = 1 + end_month = 12 + # finidat = None + finidat = "dummy_finidat" + + # try to create site + invalid_type = "INVALIDTYPE" + err_msg = ( + f"tower_type '{invalid_type}' not allowed. " + + f"Choose from: {','.join(ALLOWED_SITE_TYPES)}" + ) + with self.assertRaisesRegex(ValueError, err_msg): + TowerSite(invalid_type, name, start_year, end_year, start_month, end_month, finidat) + + +if __name__ == "__main__": + unit_testing.setup_for_tests() + unittest.main() From 196bdc07119368ffdeeb20d462c0a2ee5e49ed16 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Sat, 15 Feb 2025 10:48:04 -0700 Subject: [PATCH 21/26] Remove run_case method from tower child types. --- python/ctsm/site_and_regional/neon_site.py | 63 ------------------- python/ctsm/site_and_regional/plumber_site.py | 63 ------------------- python/ctsm/site_and_regional/run_tower.py | 8 +-- python/ctsm/site_and_regional/tower_site.py | 62 ++++++++++++------ 4 files changed, 46 insertions(+), 150 deletions(-) diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index f40ee974bf..a692896bf8 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -34,69 +34,6 @@ class NeonSite(TowerSite): A class for encapsulating neon sites. """ - # pylint: disable=too-many-statements - def run_case( - self, - base_case_root, - run_type, - prism, - user_version, - user_mods_dirs=None, - overwrite=False, - setup_only=False, - no_batch=False, - rerun=False, - experiment=False, - no_input_data_check=False, - xmlchange=None, - ): - """ - Run case. - - Args: - self - base_case_root: str, opt - file path of base case - run_type: str, opt - transient, post_ad, or ad case, default transient - prism: bool, opt - if True, use PRISM precipitation, default False - user_version: str, opt - default 'latest' - overwrite: bool, opt - default False - setup_only: bool, opt - default False; if True, set up but do not run case - no_batch: bool, opt - default False - rerun: bool, opt - default False - experiment: str, opt - name of experiment, default False - no_input_data_check: bool, opt - default False - """ - user_mods_dirs = [ - os.path.join( - self.cesmroot, "cime_config", "usermods_dirs", "clm", self.tower_type, self.name - ) - ] - - super().run_case( - base_case_root, - run_type, - prism, - user_version, - user_mods_dirs, - overwrite, - setup_only, - no_batch, - rerun, - experiment, - no_input_data_check, - xmlchange, - ) - def modify_user_nl(self, case_root, run_type, rundir, site_lines=None): # TODO: include neon-specific user namelist lines, using this as just an example currently if site_lines is None: diff --git a/python/ctsm/site_and_regional/plumber_site.py b/python/ctsm/site_and_regional/plumber_site.py index c8d7f2fc59..06174c54ef 100755 --- a/python/ctsm/site_and_regional/plumber_site.py +++ b/python/ctsm/site_and_regional/plumber_site.py @@ -33,69 +33,6 @@ class Plumber2Site(TowerSite): A class for encapsulating plumber sites. """ - # pylint: disable=too-many-statements - def run_case( - self, - base_case_root, - run_type, - prism, - user_version, - user_mods_dirs=None, - overwrite=False, - setup_only=False, - no_batch=False, - rerun=False, - experiment=False, - no_input_data_check=False, - xmlchange=None, - ): - """ - Run case. - - Args: - self - base_case_root: str, opt - file path of base case - run_type: str, opt - transient, post_ad, or ad case, default ad - (ad case is default because PLUMBER requires spinup) - prism: bool, opt - if True, use PRISM precipitation, default False - Note: only supported for NEON sites - user_version: str, opt - default 'latest'; this could be useful later - This is currently only implemented with neon (not plumber) sites - overwrite: bool, opt - default False - setup_only: bool, opt - default False; if True, set up but do not run case - no_batch: bool, opt - default False - rerun: bool, opt - default False - experiment: str, opt - name of experiment, default False - """ - user_mods_dirs = [ - os.path.join( - self.cesmroot, "cime_config", "usermods_dirs", "clm", self.tower_type, self.name - ) - ] - super().run_case( - base_case_root, - run_type, - prism, - user_version, - user_mods_dirs, - overwrite, - setup_only, - no_batch, - rerun, - experiment, - no_input_data_check, - xmlchange, - ) - def set_ref_case(self, case): super().set_ref_case(case) return True ### Check if super returns false, if this will still return True? diff --git a/python/ctsm/site_and_regional/run_tower.py b/python/ctsm/site_and_regional/run_tower.py index 5a1b50c6d1..4e10fb80fd 100755 --- a/python/ctsm/site_and_regional/run_tower.py +++ b/python/ctsm/site_and_regional/run_tower.py @@ -265,9 +265,9 @@ def main(description): if run_from_postad: neon_site.finidat = None if not base_case_root: - user_mods_dirs = None + neon_site.set_default_user_mods_dirs() base_case_root = neon_site.build_base_case( - cesmroot, output_root, res, compset, user_mods_dirs, overwrite, setup_only + cesmroot, output_root, res, compset, overwrite, setup_only ) logger.info("-----------------------------------") logger.info("Running CTSM for neon site : %s", neon_site.name) @@ -296,9 +296,9 @@ def main(description): if run_from_postad: plumber_site.finidat = None if not base_case_root: - user_mods_dirs = None + plumber_site.set_default_user_mods_dirs() base_case_root = plumber_site.build_base_case( - cesmroot, output_root, res, compset, user_mods_dirs, overwrite, setup_only + cesmroot, output_root, res, compset, overwrite, setup_only ) logger.info("-----------------------------------") logger.info("Running CTSM for plumber site : %s", plumber_site.name) diff --git a/python/ctsm/site_and_regional/tower_site.py b/python/ctsm/site_and_regional/tower_site.py index ccff184fd7..f6d387178b 100644 --- a/python/ctsm/site_and_regional/tower_site.py +++ b/python/ctsm/site_and_regional/tower_site.py @@ -42,7 +42,17 @@ class TowerSite: ------- """ - def __init__(self, tower_type, name, start_year, end_year, start_month, end_month, finidat): + def __init__( + self, + tower_type, + name, + start_year, + end_year, + start_month, + end_month, + finidat, + user_mods_dirs=None, + ): """ Initializes TowerSite with the given arguments. Parameters @@ -62,6 +72,14 @@ def __init__(self, tower_type, name, start_year, end_year, start_month, end_mont self.cesmroot = path_to_ctsm_root() self.finidat = finidat + if user_mods_dirs is None: + self.set_default_user_mods_dirs() + elif not isinstance(user_mods_dirs, list): + abort("Input user_mods_dirs is NOT a list as expected: " + str(user_mods_dirs)) + else: + self.user_mods_dirs = user_mods_dirs + self.check_user_mods_dirs() + def __str__(self): """ Converts ingredients of the TowerSite to string for printing. @@ -76,10 +94,29 @@ def __str__(self): ), ) + def set_default_user_mods_dirs(self): + """ + Sets user_mods_dirs to the default + """ + self.user_mods_dirs = [ + os.path.join( + self.cesmroot, "cime_config", "usermods_dirs", "clm", self.tower_type, self.name + ) + ] + self.check_user_mods_dirs() + + def check_user_mods_dirs(self): + """ + Checks that every user_mod_dir exists + """ + for dirtree in self.user_mods_dirs: + if not os.path.isdir(dirtree): + abort("Input user_mods_dirs dirtreetory does NOT exist: " + str(dirtree)) + # TODO: Refactor to shorten this so the disable can be removed # pylint: disable=too-many-statements def build_base_case( - self, cesmroot, output_root, res, compset, user_mods_dirs, overwrite=False, setup_only=False + self, cesmroot, output_root, res, compset, overwrite=False, setup_only=False ): """ Function for building a base_case to clone. @@ -98,20 +135,11 @@ def build_base_case( base_case resolution or gridname compset (str): base case compset - user_mods_dirs (str): - path to the user-mod-directory to use overwrite (bool) : Flag to overwrite the case if exists setup_only (bool) : Flag to only do the setup phase """ - # Define fallback user_mods_dirs - if user_mods_dirs is None: - user_mods_dirs = [ - os.path.join( - self.cesmroot, "cime_config", "usermods_dirs", "clm", self.tower_type, self.name - ) - ] # # Error checking on the input # @@ -125,11 +153,6 @@ def build_base_case( abort("Input compset is NOT a boolean as expected: " + str(compset)) if not isinstance(setup_only, bool): abort("Input setup_only is NOT a boolean as expected: " + str(setup_only)) - if user_mods_dirs is not None and not isinstance(user_mods_dirs, list): - abort("Input user_mods_dirs is NOT a list as expected: " + str(user_mods_dirs)) - for dirtree in user_mods_dirs: - if not os.path.isdir(dirtree): - abort("Input user_mods_dirs dirtreetory does NOT exist: " + str(dirtree)) print("---- building a base case -------") # pylint: disable=attribute-defined-outside-init @@ -145,7 +168,7 @@ def build_base_case( print(case_path) logger.info("base_case_name : %s", self.name) - logger.info("user_mods_dir : %s", user_mods_dirs[0]) + logger.info("user_mods_dir : %s", self.user_mods_dirs[0]) if overwrite and os.path.isdir(case_path): print("Removing the existing case at: {}".format(case_path)) @@ -165,7 +188,7 @@ def build_base_case( run_unsupported=True, answer="r", output_root=output_root, - user_mods_dirs=user_mods_dirs, + user_mods_dirs=self.user_mods_dirs, driver="nuopc", ) @@ -304,7 +327,6 @@ def run_case( run_type, prism, user_version, - user_mods_dirs, overwrite, setup_only, no_batch, @@ -412,7 +434,7 @@ def run_case( # that the shell_commands file is copied, as well as taking care of the DATM inputs. # See https://github.com/ESCOMP/CTSM/pull/1872#pullrequestreview-1169407493 # - basecase.create_clone(case_root, keepexe=True, user_mods_dirs=user_mods_dirs) + basecase.create_clone(case_root, keepexe=True, user_mods_dirs=self.user_mods_dirs) with Case(case_root, read_only=False) as case: if run_type != "transient": From 4051ee0183cbcfcc77051cbb20c4f40960d20fd2 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Sat, 15 Feb 2025 12:20:24 -0700 Subject: [PATCH 22/26] Undo a change to a help message. --- python/ctsm/site_and_regional/tower_arg_parse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ctsm/site_and_regional/tower_arg_parse.py b/python/ctsm/site_and_regional/tower_arg_parse.py index 29d04b710a..33b3db2afa 100644 --- a/python/ctsm/site_and_regional/tower_arg_parse.py +++ b/python/ctsm/site_and_regional/tower_arg_parse.py @@ -58,7 +58,7 @@ def get_parser(args, description, valid_neon_sites, valid_plumber_sites): "--base-case", help=""" Root Directory of base case build - [default: CESMROOT/NEONSITE] + [default: %(default)s] """, action="store", dest="base_case_root", From c7f70ae036b1dd605a64c2be44ad65714e5cccb2 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Sat, 15 Feb 2025 15:28:24 -0700 Subject: [PATCH 23/26] Add simple __init__() method to tower child types. --- python/ctsm/site_and_regional/neon_site.py | 3 +++ python/ctsm/site_and_regional/plumber_site.py | 3 +++ python/ctsm/site_and_regional/run_tower.py | 6 ++---- python/ctsm/test/test_unit_neon_site.py | 12 ++++++------ 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/python/ctsm/site_and_regional/neon_site.py b/python/ctsm/site_and_regional/neon_site.py index a692896bf8..da979e0be3 100755 --- a/python/ctsm/site_and_regional/neon_site.py +++ b/python/ctsm/site_and_regional/neon_site.py @@ -34,6 +34,9 @@ class NeonSite(TowerSite): A class for encapsulating neon sites. """ + def __init__(self, *args, **kwargs): + super().__init__("NEON", *args, **kwargs) + def modify_user_nl(self, case_root, run_type, rundir, site_lines=None): # TODO: include neon-specific user namelist lines, using this as just an example currently if site_lines is None: diff --git a/python/ctsm/site_and_regional/plumber_site.py b/python/ctsm/site_and_regional/plumber_site.py index 06174c54ef..2d6e006b07 100755 --- a/python/ctsm/site_and_regional/plumber_site.py +++ b/python/ctsm/site_and_regional/plumber_site.py @@ -33,6 +33,9 @@ class Plumber2Site(TowerSite): A class for encapsulating plumber sites. """ + def __init__(self, *args, **kwargs): + super().__init__("PLUMBER2", *args, **kwargs) + def set_ref_case(self, case): super().set_ref_case(case) return True ### Check if super returns false, if this will still return True? diff --git a/python/ctsm/site_and_regional/run_tower.py b/python/ctsm/site_and_regional/run_tower.py index 4e10fb80fd..fe3859fa3c 100755 --- a/python/ctsm/site_and_regional/run_tower.py +++ b/python/ctsm/site_and_regional/run_tower.py @@ -160,9 +160,7 @@ def parse_neon_listing(listing_file, valid_neon_sites): if site_name in line: finidat = line.split(",")[0].split("/")[-1] - neon_site = NeonSite( - "NEON", site_name, start_year, end_year, start_month, end_month, finidat - ) + neon_site = NeonSite(site_name, start_year, end_year, start_month, end_month, finidat) logger.debug(neon_site) available_list.append(neon_site) @@ -194,7 +192,7 @@ def setup_plumber_data(valid_plumber_sites): finidat = None plumber_site = Plumber2Site( - "PLUMBER2", site_name, start_year, end_year, start_month, end_month, finidat + site_name, start_year, end_year, start_month, end_month, finidat ) available_list.append(plumber_site) diff --git a/python/ctsm/test/test_unit_neon_site.py b/python/ctsm/test/test_unit_neon_site.py index f578f02b82..909bd3e3c0 100755 --- a/python/ctsm/test/test_unit_neon_site.py +++ b/python/ctsm/test/test_unit_neon_site.py @@ -62,9 +62,9 @@ def test_modify_user_nl_transient(self): rundir = "" # create NeonSite object and update namelist - NeonSite( - "NEON", name, start_year, end_year, start_month, end_month, finidat - ).modify_user_nl(case_root, run_type, rundir) + NeonSite(name, start_year, end_year, start_month, end_month, finidat).modify_user_nl( + case_root, run_type, rundir + ) # gather file contents for test new_nl_file = open(glob.glob(case_root + "/*")[0], "r") @@ -97,9 +97,9 @@ def test_modify_user_nl_ad(self): rundir = "" # create NeonSite object and update namelist - NeonSite( - "NEON", name, start_year, end_year, start_month, end_month, finidat - ).modify_user_nl(case_root, run_type, rundir) + NeonSite(name, start_year, end_year, start_month, end_month, finidat).modify_user_nl( + case_root, run_type, rundir + ) # gather file contents for test new_nl_file = open(glob.glob(case_root + "/*")[0], "r") From 491507a22eb152f37179b3985a1f568b6ab059e8 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Tue, 18 Feb 2025 15:37:17 -0700 Subject: [PATCH 24/26] Remove some unneeded print statements. --- python/ctsm/site_and_regional/tower_site.py | 1 - python/ctsm/test/test_sys_run_tower.py | 6 ------ 2 files changed, 7 deletions(-) diff --git a/python/ctsm/site_and_regional/tower_site.py b/python/ctsm/site_and_regional/tower_site.py index f03e56d819..aad6e0203c 100644 --- a/python/ctsm/site_and_regional/tower_site.py +++ b/python/ctsm/site_and_regional/tower_site.py @@ -128,7 +128,6 @@ def build_base_case( abort("Input output_root directory does NOT exist: " + str(output_root)) case_path = os.path.join(output_root, self.name) - print(case_path) logger.info("base_case_name : %s", self.name) logger.info("user_mods_dir : %s", user_mods_dirs[0]) diff --git a/python/ctsm/test/test_sys_run_tower.py b/python/ctsm/test/test_sys_run_tower.py index 19c3d5701a..04a318b46a 100755 --- a/python/ctsm/test/test_sys_run_tower.py +++ b/python/ctsm/test/test_sys_run_tower.py @@ -69,7 +69,6 @@ def test_one_site(self): "--output-root", self._tempdir, ] - print(sys.argv) main("") # assert that BART directories were created during setup @@ -138,7 +137,6 @@ def test_xmlchange_neon(self): "--output-root", self._tempdir, ] - print(sys.argv) main("") # Check that the --xmlchange argument is obeyed @@ -200,7 +198,6 @@ def test_setup_only_neon(self): "--output-root", self._tempdir, ] - print(sys.argv) main("") # make sure that build didn't happen: this dir should be empty @@ -251,7 +248,6 @@ def test_overwrite_neon(self): "--output-root", self._tempdir, ] - print(sys.argv) main("") # create a file that should be erased during the upcoming overwrite @@ -262,7 +258,6 @@ def test_overwrite_neon(self): # run the tool again, overwriting existing sys.argv += ["--overwrite"] - print(sys.argv) main("") # ensure that file we created is gone @@ -295,7 +290,6 @@ def test_overwrite_plumber(self): # run the tool again, overwriting existing sys.argv += ["--overwrite"] - print(sys.argv) main("") # ensure that file we created is gone From a91c855cb536c5de13a975985b73ce895dc1a8e9 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Tue, 18 Feb 2025 16:53:13 -0700 Subject: [PATCH 25/26] Remove check for allowed site types. --- python/ctsm/site_and_regional/tower_site.py | 7 --- python/ctsm/test/test_unit_tower_site.py | 70 --------------------- 2 files changed, 77 deletions(-) delete mode 100755 python/ctsm/test/test_unit_tower_site.py diff --git a/python/ctsm/site_and_regional/tower_site.py b/python/ctsm/site_and_regional/tower_site.py index 3e4d4d4ad1..7bdf47df88 100644 --- a/python/ctsm/site_and_regional/tower_site.py +++ b/python/ctsm/site_and_regional/tower_site.py @@ -29,8 +29,6 @@ logger = logging.getLogger(__name__) -ALLOWED_SITE_TYPES = ["NEON", "PLUMBER2"] - # pylint: disable=too-many-instance-attributes class TowerSite: """ @@ -58,11 +56,6 @@ def __init__( Parameters ---------- """ - if tower_type not in ALLOWED_SITE_TYPES: - raise ValueError( - f"tower_type '{tower_type}' not allowed. " - f"Choose from: {','.join(ALLOWED_SITE_TYPES)}" - ) self.tower_type = tower_type self.name = name self.start_year = int(start_year) diff --git a/python/ctsm/test/test_unit_tower_site.py b/python/ctsm/test/test_unit_tower_site.py deleted file mode 100755 index 5310e2eda1..0000000000 --- a/python/ctsm/test/test_unit_tower_site.py +++ /dev/null @@ -1,70 +0,0 @@ -#!/usr/bin/env python3 -""" -Unit tests for tower_site that aren't specific to a single tower type - -You can run this by: - python -m unittest test_unit_tower_site.py -""" - -import unittest -import tempfile -import shutil -import os -import sys - -# -- add python/ctsm to path (needed if we want to run the test stand-alone) -_CTSM_PYTHON = os.path.join(os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir) -sys.path.insert(1, _CTSM_PYTHON) - -# pylint: disable=wrong-import-position -from ctsm import unit_testing -from ctsm.site_and_regional.tower_site import TowerSite, ALLOWED_SITE_TYPES - -# pylint: disable=invalid-name - - -class Test_tower_site(unittest.TestCase): - """ - Basic class for testing tower_site.py for things not specific to any tower type. - """ - - def setUp(self): - """ - Make /_tempdir for use by these tests. - """ - self._previous_dir = os.getcwd() - self._tempdir = tempfile.mkdtemp() - - def tearDown(self): - """ - Remove temporary directory - """ - os.chdir(self._previous_dir) - shutil.rmtree(self._tempdir, ignore_errors=True) - - def test_disallowed_tower_types(self): - """ - Test that an invalid tower type throws an error - """ - # NeonSite parameters (not that it matters) - name = "ABBY" - start_year = 2020 - end_year = 2021 - start_month = 1 - end_month = 12 - # finidat = None - finidat = "dummy_finidat" - - # try to create site - invalid_type = "INVALIDTYPE" - err_msg = ( - f"tower_type '{invalid_type}' not allowed. " - + f"Choose from: {','.join(ALLOWED_SITE_TYPES)}" - ) - with self.assertRaisesRegex(ValueError, err_msg): - TowerSite(invalid_type, name, start_year, end_year, start_month, end_month, finidat) - - -if __name__ == "__main__": - unit_testing.setup_for_tests() - unittest.main() From 95f2da3bd287e6fbb8158b422f102c456d88e8a7 Mon Sep 17 00:00:00 2001 From: Erik Kluzek Date: Wed, 26 Feb 2025 10:47:54 -0700 Subject: [PATCH 26/26] Add change files --- doc/ChangeLog | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++ doc/ChangeSum | 1 + 2 files changed, 85 insertions(+) diff --git a/doc/ChangeLog b/doc/ChangeLog index 2e7e95f9b8..3a47cc5c20 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,4 +1,88 @@ =============================================================== +Tag name: ctsm5.3.026 +Originator(s): erik (Erik Kluzek,UCAR/TSS,303-497-1326) +Date: Wed 26 Feb 2025 09:55:48 AM MST +One-line Summary: Merge b4b-dev to master: run_tower updates, reduce log noise + +Purpose and description of changes +---------------------------------- + +Merge b4b-dev to master. + +Some run_tower updatesm, fix some issues, and improve code health. Also reduces some log noise. + + +Significant changes to scientifically-supported configurations +-------------------------------------------------------------- + +Does this tag change answers significantly for any of the following physics configurations? +(Details of any changes will be given in the "Answer changes" section below.) + + [Put an [X] in the box for any configuration with significant answer changes.] + +[ ] clm6_0 + +[ ] clm5_0 + +[ ] ctsm5_0-nwp + +[ ] clm4_5 + + +Bugs fixed +---------- +List of CTSM issues fixed (include CTSM Issue # and description) [one per line]: + Resolves #2884 -- run_neon overwrite ignored + Resolves #2885 -- run_neon setup_only ignored + Resolves #2946 -- python tests much longer + Resolves #2717 -- cleanup cesm log + Resolves #2737 -- reduce log noise + +Notes of particular relevance for users +--------------------------------------- +Changes to CTSM's user interface (e.g., new/renamed XML or namelist variables): + Two new options to run_tower: + --no-inputdata-check option implies --setup-only but skips the check/download of input data. This is used instead of --setup-only + in run_tower system testing for a speedup of ~20%. + --xmlchange option allows user to specify xmlchange settings to apply. E.g., --xmlchange CLM_CO2_TYPE=constant,CCSM_CO2_PPMV=850. + +Notes of particular relevance for developers: +--------------------------------------------- + +Caveats for developers (e.g., code that is duplicated that requires double maintenance): + +Testing summary: Regular +---------------- + [PASS means all tests PASS; OK means tests PASS other than expected fails.] + + python testing (if python code has changed; see instructions in python/README.md; document testing done): + + derecho - PASS + + regular tests (aux_clm: https://github.com/ESCOMP/CTSM/wiki/System-Testing-Guide#pre-merge-system-testing): + + derecho ----- OK + izumi ------- OK + +If the tag used for baseline comparisons was NOT the previous tag, note that here: + + +Answer changes +-------------- + +Changes answers relative to baseline: No bit-for-bit + +Other details +------------- + +Pull Requests that document the changes (include PR ids): +(https://github.com/ESCOMP/ctsm/pull) + #2969 -- Simplify tower types + #2962 -- Run tower fix and improvements + #2927 -- Remove log noise + +=============================================================== +=============================================================== Tag name: ctsm5.3.025 Originator(s): glemieux (Gregory Lemieux, LBNL, glemieux@lbl.gov) Date: Thu Feb 20 14:24:45 MST 2025 diff --git a/doc/ChangeSum b/doc/ChangeSum index 44d19c635e..bda8a4f301 100644 --- a/doc/ChangeSum +++ b/doc/ChangeSum @@ -1,5 +1,6 @@ Tag Who Date Summary ============================================================================================================================ + ctsm5.3.026 samrabin 02/26/2025 Merge b4b-dev to master: run_tower updates, reduce log noise ctsm5.3.025 glemieux 02/20/2025 FATES default parameter file update ctsm5.3.024 xinchang 02/11/2025 Change choice of pressure in CLMU building energy model ctsm5.3.023 afoster 02/08/2025 merge b4b-dev