From 64c29286d20b86c52ae24209d1d0fa15d921ea97 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Wed, 12 Oct 2022 06:08:46 +0000 Subject: [PATCH 01/26] Initial logging cleanup in generate_FV3LAM_wflow.py - Introduce logging handler that prints messages to screen and log file - Remove unnecessary subprocess calls, everything can now be handled pythonically through the logging module - Start removing calls to print_info_msg and print_err_msg_exit; we want to raise and properly handle exceptions, not use sys.exit - Remove/condense outdated and/or overly verbose comments - Update unit tests for new calling structure --- ush/generate_FV3LAM_wflow.py | 207 ++++++++++++----------------------- 1 file changed, 68 insertions(+), 139 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 63be16bd50..35aeb4bb46 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -5,6 +5,7 @@ import platform import subprocess import unittest +import logging from multiprocessing import Process from textwrap import dedent from datetime import datetime, timedelta @@ -61,12 +62,12 @@ def python_error_handler(): python_error_handler() -def generate_FV3LAM_wflow(): +def generate_FV3LAM_wflow(logfile: str = 'log.generate_FV3LAM_wflow') -> None: """Function to setup a forecast experiment and create a workflow - (according to the parameters specified in the config file + (according to the parameters specified in the config file) Args: - None + logfile (str): The name of the file where logging is written Returns: None """ @@ -90,14 +91,15 @@ def generate_FV3LAM_wflow(): # check python version major, minor, patch = platform.python_version_tuple() if int(major) < 3 or int(minor) < 6: - print_info_msg( + logging.error( f""" Error: python version must be 3.6 or higher python version: {major}.{minor}""" ) + raise - # define macros + # define utilities define_macos_utilities() # @@ -143,7 +145,7 @@ def generate_FV3LAM_wflow(): template_xml_fp = os.path.join(PARMdir, WFLOW_XML_FN) - print_info_msg( + logging.info( f''' Creating rocoto workflow XML file (WFLOW_XML_FP) from jinja template XML file (template_xml_fp): @@ -438,7 +440,7 @@ def generate_FV3LAM_wflow(): # End of "settings" variable. settings_str = cfg_to_yaml_str(settings) - print_info_msg( + logging.info( dedent( f""" The variable \"settings\" specifying values of the rococo XML variables @@ -447,7 +449,6 @@ def generate_FV3LAM_wflow(): settings =\n\n""" ) + settings_str, - verbose=VERBOSE, ) # @@ -459,7 +460,7 @@ def generate_FV3LAM_wflow(): ["-q", "-u", settings_str, "-t", template_xml_fp, "-o", WFLOW_XML_FP] ) except: - print_err_msg_exit( + logging.exception( dedent( f""" Call to python script fill_jinja_template.py to create a rocoto workflow @@ -482,7 +483,7 @@ def generate_FV3LAM_wflow(): # # ----------------------------------------------------------------------- # - print_info_msg( + logging.info( f''' Creating symlink in the experiment directory (EXPTDIR) that points to the workflow launch script (WFLOW_LAUNCH_SCRIPT_FP): @@ -521,12 +522,11 @@ def generate_FV3LAM_wflow(): # if SYMLINK_FIX_FILES: - print_info_msg( + logging.info( f''' Symlinking fixed files from system directory (FIXgsm) to a subdirectory (FIXam): FIXgsm = \"{FIXgsm}\" FIXam = \"{FIXam}\"''', - verbose=VERBOSE, ) ln_vrfy(f'''-fsn "{FIXgsm}" "{FIXam}"''') @@ -535,12 +535,11 @@ def generate_FV3LAM_wflow(): # else: - print_info_msg( + logging.info( f''' Copying fixed files from system directory (FIXgsm) to a subdirectory (FIXam): FIXgsm = \"{FIXgsm}\" FIXam = \"{FIXam}\"''', - verbose=VERBOSE, ) check_for_preexist_dir_file(FIXam, "delete") @@ -559,14 +558,13 @@ def generate_FV3LAM_wflow(): # ----------------------------------------------------------------------- # if USE_MERRA_CLIMO: - print_info_msg( + logging.info( f''' Copying MERRA2 aerosol climatology data files from system directory (FIXaer/FIXlut) to a subdirectory (FIXclim) in the experiment directory: FIXaer = \"{FIXaer}\" FIXlut = \"{FIXlut}\" FIXclim = \"{FIXclim}\"''', - verbose=VERBOSE, ) check_for_preexist_dir_file(FIXclim, "delete") @@ -585,30 +583,26 @@ def generate_FV3LAM_wflow(): # # ----------------------------------------------------------------------- # - print_info_msg( + logging.info( f""" Copying templates of various input files to the experiment directory...""", - verbose=VERBOSE, ) - print_info_msg( + logging.info( f""" Copying the template data table file to the experiment directory...""", - verbose=VERBOSE, ) cp_vrfy(DATA_TABLE_TMPL_FP, DATA_TABLE_FP) - print_info_msg( + logging.info( f""" Copying the template field table file to the experiment directory...""", - verbose=VERBOSE, ) cp_vrfy(FIELD_TABLE_TMPL_FP, FIELD_TABLE_FP) - print_info_msg( + logging.info( f""" Copying the template NEMS configuration file to the experiment directory...""", - verbose=VERBOSE, ) cp_vrfy(NEMS_CONFIG_TMPL_FP, NEMS_CONFIG_FP) # @@ -616,11 +610,10 @@ def generate_FV3LAM_wflow(): # clone of the FV3 code repository to the experiment directory (EXPT- # DIR). # - print_info_msg( + logging.info( f""" Copying the CCPP physics suite definition XML file from its location in the forecast model directory sturcture to the experiment directory...""", - verbose=VERBOSE, ) cp_vrfy(CCPP_PHYS_SUITE_IN_CCPP_FP, CCPP_PHYS_SUITE_FP) # @@ -628,11 +621,10 @@ def generate_FV3LAM_wflow(): # clone of the FV3 code repository to the experiment directory (EXPT- # DIR). # - print_info_msg( + logging.info( f""" Copying the field dictionary file from its location in the forecast model directory sturcture to the experiment directory...""", - verbose=VERBOSE, ) cp_vrfy(FIELD_DICT_IN_UWM_FP, FIELD_DICT_FP) # @@ -642,7 +634,7 @@ def generate_FV3LAM_wflow(): # # ----------------------------------------------------------------------- # - print_info_msg( + logging.info( f''' Setting parameters in weather model's namelist file (FV3_NML_FP): FV3_NML_FP = \"{FV3_NML_FP}\"''' @@ -877,7 +869,7 @@ def generate_FV3LAM_wflow(): settings_str = cfg_to_yaml_str(settings) - print_info_msg( + logging.info( dedent( f""" The variable \"settings\" specifying values of the weather model's @@ -886,7 +878,6 @@ def generate_FV3LAM_wflow(): settings =\n\n""" ) + settings_str, - verbose=VERBOSE, ) # # ----------------------------------------------------------------------- @@ -917,7 +908,7 @@ def generate_FV3LAM_wflow(): ] ) except: - print_err_msg_exit( + logging.exception( dedent( f""" Call to python script set_namelist.py to generate an FV3 namelist file @@ -975,7 +966,7 @@ def generate_FV3LAM_wflow(): rocotorun_cmd = f"rocotorun -w {WFLOW_XML_FN} -d {wflow_db_fn} -v 10" rocotostat_cmd = f"rocotostat -w {WFLOW_XML_FN} -d {wflow_db_fn} -v 10" - print_info_msg( + logging.info( f""" ======================================================================== ======================================================================== @@ -997,7 +988,7 @@ def generate_FV3LAM_wflow(): # if WORKFLOW_MANAGER == "rocoto": - print_info_msg( + logging.info( f""" To launch the workflow, first ensure that you have a compatible version of rocoto available. For most pre-configured platforms, rocoto can be @@ -1056,133 +1047,71 @@ def generate_FV3LAM_wflow(): {date_to_str(DATE_FIRST_CYCL,format="%H")} {NOMADS_file_type} {FCST_LEN_HRS} {LBC_SPEC_INTVL_HRS}""" ) + # If we got to this point everything was successful: move the log file to the experiment directory. + mv_vrfy(logfile, exptdir) + +def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: + """ + Sets up logging, printing high-priority (INFO and higher) messages to screen, and printing all + messages with detailed timing and routine info in the specified text file. + """ + logger = logging.getLogger() + logging.basicConfig(level=logging.DEBUG, + format='%(asctime)s %(name)-12s %(levelname)-8s %(message)s', + filename=logfile, + filemode='w') + logging.debug(f'Finished setting up debug file logging in {logfile}') + console = logging.StreamHandler() + console.setLevel(logging.INFO) + logging.getLogger().addHandler(console) + logging.debug('Logging set up successfully') -# -# ----------------------------------------------------------------------- -# -# Start of the script that will call the experiment/workflow generation -# function defined above. -# -# ----------------------------------------------------------------------- -# if __name__ == "__main__": - # - # ----------------------------------------------------------------------- - # - # Set directories. - # - # ----------------------------------------------------------------------- - # + USHdir = os.path.dirname(os.path.abspath(__file__)) - # - # Set the name of and full path to the temporary file in which we will - # save some experiment/workflow variables. The need for this temporary - # file is explained below. - # - tmp_fn = "tmp" - tmp_fp = os.path.join(USHdir, tmp_fn) - rm_vrfy("-f", tmp_fp) - # - # Set the name of and full path to the log file in which the output from - # the experiment/workflow generation function will be saved. - # - log_fn = "log.generate_FV3LAM_wflow" - log_fp = os.path.join(USHdir, log_fn) - rm_vrfy("-f", log_fp) - # + logfile='log.generate_FV3LAM_wflow' + + # Quit if not run from ush/ directory; this simplifies so much logic + if USHdir != os.getcwd(): + print(f"This script must be run from the ush/ directory\n{USHdir}") + sys.exit(1) + + # Set the log filename; this will be written in USHdir during this script's + # execution and moved to EXPTDIR once the experiment is successfully created + setup_logging(logfile) + # Call the generate_FV3LAM_wflow function defined above to generate the - # experiment/workflow. Note that we pipe the output of the function - # (and possibly other commands) to the "tee" command in order to be able - # to both save it to a file and print it out to the screen (stdout). - # The piping causes the call to the function (and the other commands - # grouped with it using the curly braces, { ... }) to be executed in a - # subshell. As a result, the experiment/workflow variables that the - # function sets are not available outside of the grouping, i.e. they are - # not available at and after the call to "tee". Since some of these va- - # riables are needed after the call to "tee" below, we save them in a - # temporary file and read them in outside the subshell later below. - # - def workflow_func(): - retval = 1 - generate_FV3LAM_wflow() - retval = 0 - run_command(f'''echo "{EXPTDIR}" >> "{tmp_fp}"''') - run_command(f'''echo "{retval}" >> "{tmp_fp}"''') - - # create tee functionality - tee = subprocess.Popen(["tee", log_fp], stdin=subprocess.PIPE) - os.dup2(tee.stdin.fileno(), sys.stdout.fileno()) - os.dup2(tee.stdin.fileno(), sys.stderr.fileno()) - - # create workflow process - p = Process(target=workflow_func) - p.start() - p.join() - - # - # Read in experiment/workflow variables needed later below from the tem- - # porary file created in the subshell above containing the call to the - # generate_FV3LAM_wflow function. These variables are not directly - # available here because the call to generate_FV3LAM_wflow above takes - # place in a subshell (due to the fact that we are then piping its out- - # put to the "tee" command). Then remove the temporary file. - # - (_, exptdir, _) = run_command(f'''sed "1q;d" "{tmp_fp}"''') - (_, retval, _) = run_command(f''' sed "2q;d" "{tmp_fp}"''') - if retval: - retval = int(retval) - else: - retval = 1 - rm_vrfy(tmp_fp) - # - # If the call to the generate_FV3LAM_wflow function above was success- - # ful, move the log file in which the "tee" command saved the output of - # the function to the experiment directory. - # - if retval == 0: - mv_vrfy(log_fp, exptdir) - # - # If the call to the generate_FV3LAM_wflow function above was not suc- - # cessful, print out an error message and exit with a nonzero return - # code. - # - else: - print_err_msg_exit( + # experiment/workflow. + try: + generate_FV3LAM_wflow(logfile) + except: + # If the call to the generate_FV3LAM_wflow function above was not successful, + # print out an error message and exit with a nonzero return code. + logging.error( f""" - Experiment generation failed. Check the log file from the ex- - periment/workflow generation script in the file specified by log_fp: - log_fp = \"{log_fp}\" + Experiment generation failed. See the error message(s) printed above. + For more detailed information, check the log file from the workflow + generation script: {logfile} Stopping.""" ) class Testing(unittest.TestCase): def test_generate_FV3LAM_wflow(self): - # run workflows in separate process to avoid conflict - def workflow_func(): - generate_FV3LAM_wflow() - - def run_workflow(): - p = Process(target=workflow_func) - p.start() - p.join() - exit_code = p.exitcode - if exit_code != 0: - sys.exit(exit_code) - USHdir = os.path.dirname(os.path.abspath(__file__)) + logfile='log.generate_FV3LAM_wflow' SED = get_env_var("SED") # community test case cp_vrfy(f"{USHdir}/config.community.yaml", f"{USHdir}/config.yaml") run_command(f"""{SED} -i 's/MACHINE: hera/MACHINE: linux/g' {USHdir}/config.yaml""") - run_workflow() + generate_FV3LAM_wflow(logfile) # nco test case set_env_var("OPSROOT", f"{USHdir}/../../nco_dirs") cp_vrfy(f"{USHdir}/config.nco.yaml", f"{USHdir}/config.yaml") run_command(f"""{SED} -i 's/MACHINE: hera/MACHINE: linux/g' {USHdir}/config.yaml""") - run_workflow() + generate_FV3LAM_wflow(logfile) def setUp(self): define_macos_utilities() From 6a077617e274f002f475974bd2555673b5638acb Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Wed, 12 Oct 2022 19:57:17 +0000 Subject: [PATCH 02/26] More updates for better logging and error messages - Fix some functionality problems - Fix extraneous VERBOSE argument that I missed - NOMADS capability is clearly broken currently (calls the wrong filename), so for now move to a function that if called, returns an exception - Fix log file move - Re-enable running from other directories, did not realize this was necessary for WE2E tests - pass USHdir to generate_FV3LAM_wflow() and handle log file moving at the end of that function - Remove time info from log file to reduce clutter - Raise exception with trace if generate_FV3LAM_wflow() fails - This returns non-zero exit code, so external behavior is the same --- ush/generate_FV3LAM_wflow.py | 85 +++++++++++++----------------------- 1 file changed, 31 insertions(+), 54 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 35aeb4bb46..56eb5b7228 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -62,32 +62,29 @@ def python_error_handler(): python_error_handler() -def generate_FV3LAM_wflow(logfile: str = 'log.generate_FV3LAM_wflow') -> None: +def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> None: """Function to setup a forecast experiment and create a workflow (according to the parameters specified in the config file) Args: + USHdir (str): The full path of the ush/ directory where this script is located logfile (str): The name of the file where logging is written Returns: None """ - print( + # Set up logging to write to screen and logfile + setup_logging(logfile) + + logging.info( dedent( """ ======================================================================== - ======================================================================== - Starting experiment generation... - - ======================================================================== ========================================================================""" ) ) - # set USHdir - USHdir = os.path.dirname(os.path.abspath(__file__)) - # check python version major, minor, patch = platform.python_version_tuple() if int(major) < 3 or int(minor) < 6: @@ -102,18 +99,8 @@ def generate_FV3LAM_wflow(logfile: str = 'log.generate_FV3LAM_wflow') -> None: # define utilities define_macos_utilities() - # - # ----------------------------------------------------------------------- - # - # Source the file that defines and then calls the setup function. The - # setup function in turn first sources the default configuration file - # (which contains default values for the experiment/workflow parameters) - # and then sources the user-specified configuration file (which contains - # user-specified values for a subset of the experiment/workflow parame- - # ters that override their default values). - # - # ----------------------------------------------------------------------- - # + # The setup function reads the user configuration file and fills in + # non-user-specified values from config_defaults.yaml setup() # import all environment variables @@ -489,7 +476,6 @@ def generate_FV3LAM_wflow(logfile: str = 'log.generate_FV3LAM_wflow') -> None: workflow launch script (WFLOW_LAUNCH_SCRIPT_FP): EXPTDIR = \"{EXPTDIR}\" WFLOW_LAUNCH_SCRIPT_FP = \"{WFLOW_LAUNCH_SCRIPT_FP}\"''', - verbose=VERBOSE, ) create_symlink_to_file( @@ -941,6 +927,11 @@ def generate_FV3LAM_wflow(logfile: str = 'log.generate_FV3LAM_wflow') -> None: if not RUN_TASK_MAKE_GRID: set_FV3nml_sfc_climo_filenames() + + # Call script to get NOMADS data + if NOMADS: + get_nomads_data(NOMADS_file_type,EXPTDIR,USHdir,DATE_FIRST_CYCL,CYCL_HRS,FCST_LEN_HRS,LBC_SPEC_INTVL_HRS) + # # ----------------------------------------------------------------------- # @@ -1034,30 +1025,26 @@ def generate_FV3LAM_wflow(logfile: str = 'log.generate_FV3LAM_wflow') -> None: */{CRON_RELAUNCH_INTVL_MNTS} * * * * cd {EXPTDIR} && ./launch_FV3LAM_wflow.sh called_from_cron=\"TRUE\" """ ) - # - # If necessary, run the NOMADS script to source external model data. - # - if NOMADS: - print("Getting NOMADS online data") - print(f"NOMADS_file_type= {NOMADS_file_type}") - cd_vrfy(EXPTDIR) - NOMADS_script = os.path.join(USHdir, "NOMADS_get_extrn_mdl_files.h") - run_command( - f"""{NOMADS_script} {date_to_str(DATE_FIRST_CYCL,format="%Y%m%d")} \ - {date_to_str(DATE_FIRST_CYCL,format="%H")} {NOMADS_file_type} {FCST_LEN_HRS} {LBC_SPEC_INTVL_HRS}""" - ) # If we got to this point everything was successful: move the log file to the experiment directory. - mv_vrfy(logfile, exptdir) + mv_vrfy(logfile, EXPTDIR) + +def get_nomads_data(NOMADS_file_type,EXPTDIR,USHdir,DATE_FIRST_CYCL,CYCL_HRS,FCST_LEN_HRS,LBC_SPEC_INTVL_HRS): + print("Getting NOMADS online data") + print(f"NOMADS_file_type= {NOMADS_file_type}") + cd_vrfy(EXPTDIR) + NOMADS_script = os.path.join(USHdir, "NOMADS_get_extrn_mdl_files.sh") + # run_command(f"""{NOMADS_script} {date_to_str(DATE_FIRST_CYCL,format="%Y%m%d")} \ + # {date_to_str(DATE_FIRST_CYCL,format="%H")} {NOMADS_file_type} {FCST_LEN_HRS} {LBC_SPEC_INTVL_HRS}""") + raise Exception("Nomads script does not work") def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: """ Sets up logging, printing high-priority (INFO and higher) messages to screen, and printing all messages with detailed timing and routine info in the specified text file. """ - logger = logging.getLogger() logging.basicConfig(level=logging.DEBUG, - format='%(asctime)s %(name)-12s %(levelname)-8s %(message)s', + format='%(name)-12s %(levelname)-8s %(message)s', filename=logfile, filemode='w') logging.debug(f'Finished setting up debug file logging in {logfile}') @@ -1069,30 +1056,20 @@ def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: if __name__ == "__main__": USHdir = os.path.dirname(os.path.abspath(__file__)) - logfile='log.generate_FV3LAM_wflow' - - # Quit if not run from ush/ directory; this simplifies so much logic - if USHdir != os.getcwd(): - print(f"This script must be run from the ush/ directory\n{USHdir}") - sys.exit(1) - - # Set the log filename; this will be written in USHdir during this script's - # execution and moved to EXPTDIR once the experiment is successfully created - setup_logging(logfile) + logfile=f'{USHdir}/log.generate_FV3LAM_wflow' # Call the generate_FV3LAM_wflow function defined above to generate the # experiment/workflow. try: - generate_FV3LAM_wflow(logfile) + generate_FV3LAM_wflow(USHdir, logfile) except: # If the call to the generate_FV3LAM_wflow function above was not successful, # print out an error message and exit with a nonzero return code. - logging.error( + logging.exception( f""" - Experiment generation failed. See the error message(s) printed above. + Experiment generation failed. See the error message(s) printed below. For more detailed information, check the log file from the workflow - generation script: {logfile} - Stopping.""" + generation script: {logfile}""" ) class Testing(unittest.TestCase): @@ -1105,13 +1082,13 @@ def test_generate_FV3LAM_wflow(self): # community test case cp_vrfy(f"{USHdir}/config.community.yaml", f"{USHdir}/config.yaml") run_command(f"""{SED} -i 's/MACHINE: hera/MACHINE: linux/g' {USHdir}/config.yaml""") - generate_FV3LAM_wflow(logfile) + generate_FV3LAM_wflow(USHdir, logfile) # nco test case set_env_var("OPSROOT", f"{USHdir}/../../nco_dirs") cp_vrfy(f"{USHdir}/config.nco.yaml", f"{USHdir}/config.yaml") run_command(f"""{SED} -i 's/MACHINE: hera/MACHINE: linux/g' {USHdir}/config.yaml""") - generate_FV3LAM_wflow(logfile) + generate_FV3LAM_wflow(USHdir, logfile) def setUp(self): define_macos_utilities() From 4e06bf00d4344dce4a01b0d48233d5b947551674 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Wed, 12 Oct 2022 23:27:02 +0000 Subject: [PATCH 03/26] Set up logging in setup.py - Pass logger to setup() - Convert calls print_info_msg into logger.info - Convert calls to print_err_msg_exit to raise exceptions (specific where appropriate), preserving the error message - Condense some overly verbose comments - Do not print success message for individual scripts; could be confusing to users --- ush/generate_FV3LAM_wflow.py | 2 +- ush/setup.py | 186 +++++++++++++---------------------- 2 files changed, 72 insertions(+), 116 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 56eb5b7228..894bf5b888 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -101,7 +101,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # The setup function reads the user configuration file and fills in # non-user-specified values from config_defaults.yaml - setup() + setup(logging.getLogger('root.setup')) # import all environment variables import_vars() diff --git a/ush/setup.py b/ush/setup.py index 4d6841be54..b3b3900403 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -4,6 +4,7 @@ import sys import datetime from textwrap import dedent +from logging import Logger, getLogger from python_utils import ( cd_vrfy, @@ -18,8 +19,6 @@ import_vars, export_vars, get_env_var, - print_info_msg, - print_err_msg_exit, load_config_file, cfg_to_shell_str, cfg_to_yaml_str, @@ -39,7 +38,7 @@ from set_thompson_mp_fix_files import set_thompson_mp_fix_files -def setup(): +def setup(logger: Logger = getLogger()): """Function that sets a secondary set of parameters needed by the various scripts that are called by the FV3-LAM rocoto community workflow. This secondary set of parameters is @@ -51,7 +50,8 @@ def setup(): scripts called by the tasks in the workflow. Args: - None + logger: A logger object for Python's built in logging module. Typically + this is set up in generate_FV3LAM_workflow and passed here. Returns: None """ @@ -64,12 +64,12 @@ def setup(): import_vars() # print message - print_info_msg( + logger.info(dedent( f""" ======================================================================== Starting function setup() in \"{os.path.basename(__file__)}\"... ========================================================================""" - ) + )) # # ----------------------------------------------------------------------- # @@ -93,19 +93,14 @@ def setup(): # # ----------------------------------------------------------------------- # - if os.path.exists(EXPT_CONFIG_FN): - cfg_u = load_config_file(os.path.join(USHdir, EXPT_CONFIG_FN)) - cfg_u = flatten_dict(cfg_u) - import_vars(dictionary=cfg_u, - env_vars=["MACHINE", - "EXTRN_MDL_NAME_ICS", "EXTRN_MDL_NAME_LBCS", - "FV3GFS_FILE_FMT_ICS", "FV3GFS_FILE_FMT_LBCS"]) - else: - print_err_msg_exit( - f''' - User config file not found - EXPT_CONFIG_FN = \"{EXPT_CONFIG_FN}\"''' - ) + if not os.path.exists(EXPT_CONFIG_FN): + raise FileNotFoundError(f'User config file not found: {EXPT_CONFIG_FN=}') + + cfg_u = load_config_file(os.path.join(USHdir, EXPT_CONFIG_FN)) + cfg_u = flatten_dict(cfg_u) + import_vars(dictionary=cfg_u, env_vars=["MACHINE", + "EXTRN_MDL_NAME_ICS", "EXTRN_MDL_NAME_LBCS", + "FV3GFS_FILE_FMT_ICS", "FV3GFS_FILE_FMT_LBCS"]) # # ----------------------------------------------------------------------- # @@ -181,7 +176,7 @@ def get_location(xcs,fmt): # global WORKFLOW_ID WORKFLOW_ID = "id_" + str(int(datetime.datetime.now().timestamp())) - print_info_msg(f"""WORKFLOW ID = {WORKFLOW_ID}""") + logger.info(f"""WORKFLOW ID = {WORKFLOW_ID}""") # # ----------------------------------------------------------------------- @@ -208,7 +203,7 @@ def get_location(xcs,fmt): # global VERBOSE if DEBUG and not VERBOSE: - print_info_msg( + logging.info( """ Resetting VERBOSE to \"TRUE\" because DEBUG has been set to \"TRUE\"...""" ) @@ -289,7 +284,7 @@ def get_location(xcs,fmt): or (len(SPP_STDDEV_CUTOFF) != N_VAR_SPP) or (len(ISEED_SPP) != N_VAR_SPP) ): - print_err_msg_exit( + raise Exception( f''' All MYNN PBL, MYNN SFC, GSL GWD, Thompson MP, or RRTMG SPP-related namelist variables set in {CONFIG_FN} must be equal in number of entries to what is @@ -311,7 +306,7 @@ def get_location(xcs,fmt): or (len(LSM_SPP_LSCALE) != N_VAR_LNDP) or (len(LSM_SPP_TSCALE) != N_VAR_LNDP) ): - print_err_msg_exit( + raise Exception( f''' All Noah or RUC-LSM SPP-related namelist variables (except ISEED_LSM_SPP) set in {CONFIG_FN} must be equal in number of entries to what is found in @@ -354,14 +349,14 @@ def get_location(xcs,fmt): UFS_WTHR_MDL_DIR = get_ini_value(cfg, external_name, property_name) if not UFS_WTHR_MDL_DIR: - print_err_msg_exit( + raise Exception( f""" Externals.cfg does not contain "{external_name}".""" ) UFS_WTHR_MDL_DIR = os.path.join(HOMEdir, UFS_WTHR_MDL_DIR) if not os.path.exists(UFS_WTHR_MDL_DIR): - print_err_msg_exit( + raise FileNotFoundError( f""" The base directory in which the FV3 source code should be located (UFS_WTHR_MDL_DIR) does not exist: @@ -401,14 +396,14 @@ def get_location(xcs,fmt): RELATIVE_LINK_FLAG = "--relative" if not NCORES_PER_NODE: - print_err_msg_exit( + raise Exception( f""" NCORES_PER_NODE has not been specified in the file {MACHINE_FILE} Please ensure this value has been set for your desired platform. """ ) if not (FIXgsm and FIXaer and FIXlut and TOPO_DIR and SFC_CLIMO_INPUT_DIR): - print_err_msg_exit( + raise Exception( f""" One or more fix file directories have not been specified for this machine: MACHINE = \"{MACHINE}\" @@ -459,7 +454,7 @@ def get_location(xcs,fmt): # if WORKFLOW_MANAGER is not None: if not ACCOUNT: - print_err_msg_exit( + raise Exception( f''' The variable ACCOUNT cannot be empty if you are using a workflow manager: ACCOUNT = \"ACCOUNT\" @@ -503,7 +498,7 @@ def get_location(xcs,fmt): elif FCST_MODEL == "fv3gfs_aqm": CPL = True else: - print_err_msg_exit( + raise Exception( f''' The coupling flag CPL has not been specified for this value of FCST_MODEL: FCST_MODEL = \"{FCST_MODEL}\"''' @@ -516,7 +511,7 @@ def get_location(xcs,fmt): # ----------------------------------------------------------------------- # if not isinstance(RESTART_INTERVAL, int): - print_err_msg_exit( + raise Exception( f''' RESTART_INTERVAL must be set to an integer number of hours. RESTART_INTERVAL = \"{RESTART_INTERVAL}\"''' @@ -530,7 +525,7 @@ def get_location(xcs,fmt): # ----------------------------------------------------------------------- # if not isinstance(DATE_FIRST_CYCL, datetime.date): - print_err_msg_exit( + raise Exception( f''' DATE_FIRST_CYCL must be a string consisting of exactly 10 digits of the form \"YYYYMMDDHH\", where YYYY is the 4-digit year, MM is the 2-digit @@ -540,7 +535,7 @@ def get_location(xcs,fmt): ) if not isinstance(DATE_LAST_CYCL, datetime.date): - print_err_msg_exit( + raise Exception( f''' DATE_LAST_CYCL must be a string consisting of exactly 10 digits of the form \"YYYYMMDDHH\", where YYYY is the 4-digit year, MM is the 2-digit @@ -571,55 +566,37 @@ def get_location(xcs,fmt): # Completely arbitrary cutoff of 90 cycles. if NUM_CYCLES > 90: ALL_CDATES = None - print_info_msg( + logger.warning( f""" Too many cycles in ALL_CDATES to list, redefining in abbreviated form." ALL_CDATES="{DATE_FIRST_CYCL}...{DATE_LAST_CYCL}""" ) - # - # ----------------------------------------------------------------------- - # + # If using a custom post configuration file, make sure that it exists. - # - # ----------------------------------------------------------------------- - # if USE_CUSTOM_POST_CONFIG_FILE: if not os.path.exists(CUSTOM_POST_CONFIG_FP): - print_err_msg_exit( + raise FileNotFoundError( f''' The custom post configuration specified by CUSTOM_POST_CONFIG_FP does not exist: CUSTOM_POST_CONFIG_FP = \"{CUSTOM_POST_CONFIG_FP}\"''' ) - # - # ----------------------------------------------------------------------- - # + # If using external CRTM fix files to allow post-processing of synthetic - # satellite products from the UPP, then make sure the fix file directory - # exists. - # - # ----------------------------------------------------------------------- - # + # satellite products from the UPP, make sure the CRTM fix file directory exists. if USE_CRTM: if not os.path.exists(CRTM_DIR): - print_err_msg_exit( + raise FileNotFoundError( f''' The external CRTM fix file directory specified by CRTM_DIR does not exist: CRTM_DIR = \"{CRTM_DIR}\"''' ) - # - # ----------------------------------------------------------------------- - # - # The forecast length (in integer hours) cannot contain more than 3 cha- - # racters. Thus, its maximum value is 999. Check whether the specified - # forecast length exceeds this maximum value. If so, print out a warn- - # ing and exit this script. - # - # ----------------------------------------------------------------------- - # + + # The forecast length (in integer hours) cannot contain more than 3 characters. + # Thus, its maximum value is 999. fcst_len_hrs_max = 999 if FCST_LEN_HRS > fcst_len_hrs_max: - print_err_msg_exit( + raise ValueError( f""" Forecast length is greater than maximum allowed length: FCST_LEN_HRS = {FCST_LEN_HRS} @@ -629,16 +606,15 @@ def get_location(xcs,fmt): # ----------------------------------------------------------------------- # # Check whether the forecast length (FCST_LEN_HRS) is evenly divisible - # by the BC update interval (LBC_SPEC_INTVL_HRS). If not, print out a - # warning and exit this script. If so, generate an array of forecast - # hours at which the boundary values will be updated. + # by the BC update interval (LBC_SPEC_INTVL_HRS). If so, generate an + # array of forecast hours at which the boundary values will be updated. # # ----------------------------------------------------------------------- # rem = FCST_LEN_HRS % LBC_SPEC_INTVL_HRS if rem != 0: - print_err_msg_exit( + raise Exception( f""" The forecast length (FCST_LEN_HRS) is not evenly divisible by the lateral boundary conditions update interval (LBC_SPEC_INTVL_HRS): @@ -673,7 +649,7 @@ def get_location(xcs,fmt): # ----------------------------------------------------------------------- # if not DT_ATMOS: - print_err_msg_exit( + raise Exception( f''' The forecast model main time step (DT_ATMOS) is set to a null string: DT_ATMOS = {DT_ATMOS} @@ -683,7 +659,7 @@ def get_location(xcs,fmt): ) if not LAYOUT_X: - print_err_msg_exit( + raise Exception( f''' The number of MPI processes to be used in the x direction (LAYOUT_X) by the forecast job is set to a null string: @@ -694,7 +670,7 @@ def get_location(xcs,fmt): ) if not LAYOUT_Y: - print_err_msg_exit( + raise Exception( f''' The number of MPI processes to be used in the y direction (LAYOUT_Y) by the forecast job is set to a null string: @@ -705,7 +681,7 @@ def get_location(xcs,fmt): ) if not BLOCKSIZE: - print_err_msg_exit( + raise Exception( f''' The cache size to use for each MPI task of the forecast (BLOCKSIZE) is set to a null string: @@ -730,7 +706,7 @@ def get_location(xcs,fmt): # Check that DT_SUBHOURLY_POST_MNTS is between 0 and 59, inclusive. # if DT_SUBHOURLY_POST_MNTS < 0 or DT_SUBHOURLY_POST_MNTS > 59: - print_err_msg_exit( + raise ValueError( f''' When performing sub-hourly post (i.e. SUB_HOURLY_POST set to \"TRUE\"), DT_SUBHOURLY_POST_MNTS must be set to an integer between 0 and 59, @@ -744,7 +720,7 @@ def get_location(xcs,fmt): # rem = DT_SUBHOURLY_POST_MNTS * 60 % DT_ATMOS if rem != 0: - print_err_msg_exit( + raise ValueError( f""" When performing sub-hourly post (i.e. SUB_HOURLY_POST set to \"TRUE\"), the time interval specified by DT_SUBHOURLY_POST_MNTS (after converting @@ -765,7 +741,7 @@ def get_location(xcs,fmt): # informational message that such a change was made. # if DT_SUBHOURLY_POST_MNTS == 0: - print_info_msg( + logger.warning( f""" When performing sub-hourly post (i.e. SUB_HOURLY_POST set to \"TRUE\"), DT_SUBHOURLY_POST_MNTS must be set to a value greater than 0; otherwise, @@ -804,16 +780,10 @@ def get_location(xcs,fmt): EXPT_BASEDIR = os.path.abspath(EXPT_BASEDIR) mkdir_vrfy(f' -p "{EXPT_BASEDIR}"') - # - # ----------------------------------------------------------------------- - # - # If the experiment subdirectory name (EXPT_SUBDIR) is set to an empty - # string, print out an error message and exit. - # - # ----------------------------------------------------------------------- - # + + # The experiment subdirectory name (EXPT_SUBDIR) can not be set to an empty string if not EXPT_SUBDIR: - print_err_msg_exit( + raise Exception( f''' The name of the experiment subdirectory (EXPT_SUBDIR) cannot be empty: EXPT_SUBDIR = \"{EXPT_SUBDIR}\"''' @@ -945,7 +915,7 @@ def get_location(xcs,fmt): if POST_OUTPUT_DOMAIN_NAME is None: if PREDEF_GRID_NAME is None: - print_err_msg_exit( + raise Exception( f""" The domain name used in naming the run_post output files (POST_OUTPUT_DOMAIN_NAME) has not been set: @@ -1056,7 +1026,7 @@ def get_location(xcs,fmt): ) CCPP_PHYS_SUITE_FP = os.path.join(EXPTDIR, CCPP_PHYS_SUITE_FN) if not os.path.exists(CCPP_PHYS_SUITE_IN_CCPP_FP): - print_err_msg_exit( + raise FileNotFoundError( f''' The CCPP suite definition file (CCPP_PHYS_SUITE_IN_CCPP_FP) does not exist in the local clone of the ufs-weather-model: @@ -1083,7 +1053,7 @@ def get_location(xcs,fmt): ) FIELD_DICT_FP = os.path.join(EXPTDIR, FIELD_DICT_FN) if not os.path.exists(FIELD_DICT_IN_UWM_FP): - print_err_msg_exit( + raise FileNotFoundError( f''' The field dictionary file (FIELD_DICT_IN_UWM_FP) does not exist in the local clone of the ufs-weather-model: @@ -1159,7 +1129,7 @@ def get_location(xcs,fmt): idx = len(EXTRN_MDL_SOURCE_BASEDIR_ICS) if not os.path.exists(EXTRN_MDL_SOURCE_BASEDIR_ICS[:idx]): - print_err_msg_exit( + raise FileNotFoundError( f''' The directory (EXTRN_MDL_SOURCE_BASEDIR_ICS) in which the user-staged external model files for generating ICs should be located does not exist: @@ -1171,7 +1141,7 @@ def get_location(xcs,fmt): idx = len(EXTRN_MDL_SOURCE_BASEDIR_LBCS) if not os.path.exists(EXTRN_MDL_SOURCE_BASEDIR_LBCS[:idx]): - print_err_msg_exit( + raise FileNotFoundError( f''' The directory (EXTRN_MDL_SOURCE_BASEDIR_LBCS) in which the user-staged external model files for generating LBCs should be located does not exist: @@ -1268,7 +1238,7 @@ def get_location(xcs,fmt): # ----------------------------------------------------------------------- # if (not DO_ENSEMBLE) and (RUN_TASK_VX_ENSGRID or RUN_TASK_VX_ENSPOINT): - print_err_msg_exit( + raise Exception( f''' Ensemble verification can not be run unless running in ensemble mode: DO_ENSEMBLE = \"{DO_ENSEMBLE}\" @@ -1320,10 +1290,10 @@ def get_location(xcs,fmt): msg = f"""Setting GRID_DIR to: GRID_DIR = \"{GRID_DIR}\" """ - print_info_msg(msg) + logger.info(msg) if not os.path.exists(GRID_DIR): - print_err_msg_exit( + raise FileNotFoundError( f''' The directory (GRID_DIR) that should contain the pregenerated grid files does not exist: @@ -1344,10 +1314,10 @@ def get_location(xcs,fmt): msg = f"""Setting OROG_DIR to: OROG_DIR = \"{OROG_DIR}\" """ - print_info_msg(msg) + logger.info(msg) if not os.path.exists(OROG_DIR): - print_err_msg_exit( + raise FileNotFoundError( f''' The directory (OROG_DIR) that should contain the pregenerated orography files does not exist: @@ -1368,10 +1338,10 @@ def get_location(xcs,fmt): msg = f"""Setting SFC_CLIMO_DIR to: SFC_CLIMO_DIR = \"{SFC_CLIMO_DIR}\" """ - print_info_msg(msg) + logger.info(msg) if not os.path.exists(SFC_CLIMO_DIR): - print_err_msg_exit( + raise FileNotFoundError( f''' The directory (SFC_CLIMO_DIR) that should contain the pregenerated surface climatology files does not exist: @@ -1520,7 +1490,7 @@ def get_location(xcs,fmt): res_in_orog_fns = link_fix(verbose=VERBOSE, file_group="orog") if not RES_IN_FIXLAM_FILENAMES and (res_in_orog_fns != RES_IN_FIXLAM_FILENAMES): - print_err_msg_exit( + raise Exception( f""" The resolution extracted from the orography file names (res_in_orog_fns) does not match the resolution in other groups of files already consi- @@ -1546,7 +1516,7 @@ def get_location(xcs,fmt): res_in_sfc_climo_fns = link_fix(verbose=VERBOSE, file_group="sfc_climo") if RES_IN_FIXLAM_FILENAMES and res_in_sfc_climo_fns != RES_IN_FIXLAM_FILENAMES: - print_err_msg_exit( + raise Exception( f""" The resolution extracted from the surface climatology file names (res_- in_sfc_climo_fns) does not match the resolution in other groups of files @@ -1583,7 +1553,7 @@ def get_location(xcs,fmt): # Check if SUB_HOURLY_POST is on if SUB_HOURLY_POST: - print_err_msg_exit( + raise Exception( f""" SUB_HOURLY_POST is NOT available with Inline Post yet.""" ) @@ -1601,12 +1571,11 @@ def get_location(xcs,fmt): if QUILTING: PE_MEMBER01 = PE_MEMBER01 + WRTCMP_write_groups * WRTCMP_write_tasks_per_group - print_info_msg( + logger.info( f""" The number of MPI tasks for the forecast (including those for the write component if it is being used) are: PE_MEMBER01 = {PE_MEMBER01}""", - verbose=VERBOSE, ) # # ----------------------------------------------------------------------- @@ -1963,11 +1932,12 @@ def get_location(xcs,fmt): # # print content of var_defns if DEBUG=True - all_lines = cfg_to_yaml_str(cfg_d) - print_info_msg(all_lines, verbose=DEBUG) + if DEBUG: + all_lines = cfg_to_yaml_str(cfg_d) + logger.info(all_lines) # print info message - print_info_msg( + logger.info( f""" Generating the global experiment variable definitions file specified by GLOBAL_VAR_DEFNS_FN: @@ -2000,27 +1970,13 @@ def get_location(xcs,fmt): continue vkey = "valid_vals_" + k if (vkey in cfg_v) and not (v in cfg_v[vkey]): - print_err_msg_exit( + raise Exception( f""" The variable {k}={v} in {EXPT_DEFAULT_CONFIG_FN} or {EXPT_CONFIG_FN} does not have a valid value. Possible values are: {k} = {cfg_v[vkey]}""" ) - # - # ----------------------------------------------------------------------- - # - # Print message indicating successful completion of script. - # - # ----------------------------------------------------------------------- - # - print_info_msg( - f""" - ======================================================================== - Function setup() in \"{os.path.basename(__file__)}\" completed successfully!!! - ========================================================================""" - ) - # # ----------------------------------------------------------------------- From b49ce0b5959ed1b0405fa00da27d2b66c4d0feda Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Thu, 13 Oct 2022 00:17:54 +0000 Subject: [PATCH 04/26] Add check that config.yaml contains only variables found in config_defaults.yaml --- ush/setup.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ush/setup.py b/ush/setup.py index b3b3900403..0b8081cab5 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -86,18 +86,19 @@ def setup(logger: Logger = getLogger()): "EXTRN_MDL_NAME_ICS", "EXTRN_MDL_NAME_LBCS", "FV3GFS_FILE_FMT_ICS", "FV3GFS_FILE_FMT_LBCS"]) - # - # ----------------------------------------------------------------------- - # - # Load the user config file but don't source it yet. - # - # ----------------------------------------------------------------------- - # + + # Load the user config file, containing contains user-specified values + # variables that override their default values. Ensure all user-specified + # variables correspond to a default value. if not os.path.exists(EXPT_CONFIG_FN): raise FileNotFoundError(f'User config file not found: {EXPT_CONFIG_FN=}') cfg_u = load_config_file(os.path.join(USHdir, EXPT_CONFIG_FN)) cfg_u = flatten_dict(cfg_u) + for key in cfg_u: + if key not in cfg_d: + raise Exception(dedent(f'''User-specified variable {key} in {EXPT_CONFIG_FN} is not valid + Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables\n''')) import_vars(dictionary=cfg_u, env_vars=["MACHINE", "EXTRN_MDL_NAME_ICS", "EXTRN_MDL_NAME_LBCS", "FV3GFS_FILE_FMT_ICS", "FV3GFS_FILE_FMT_LBCS"]) From c68d217beb9d5037ee351a283ff4855ab8e0b330 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Thu, 13 Oct 2022 00:26:58 +0000 Subject: [PATCH 05/26] Add checks for required python version and checks to run_WE2E_tests.sh --- tests/WE2E/run_WE2E_tests.sh | 57 ++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/WE2E/run_WE2E_tests.sh b/tests/WE2E/run_WE2E_tests.sh index bddf87ca83..ec06a88150 100755 --- a/tests/WE2E/run_WE2E_tests.sh +++ b/tests/WE2E/run_WE2E_tests.sh @@ -65,6 +65,63 @@ WE2Edir="$TESTSdir/WE2E" # #----------------------------------------------------------------------- # +# Run python checks +# +#----------------------------------------------------------------------- +# + +# This line will return two numbers: the python major and minor versions +pyversion=($(/usr/bin/env python3 -c 'import platform; major, minor, patch = platform.python_version_tuple(); print(major); print(minor)')) + +#Now, set an error check variable so that we can print all python errors rather than just the first +pyerrors=0 + +# Check that the call to python3 returned no errors, then check if the +# python3 minor version is 6 or higher +if [[ -z "$pyversion" ]];then + print_info_msg "\ + + Error: python3 not found" + pyerrors=$((pyerrors+1)) +else + if [[ ${#pyversion[@]} -lt 2 ]]; then + print_info_msg "\ + + Error retrieving python3 version" + pyerrors=$((pyerrors+1)) + elif [[ ${pyversion[1]} -lt 6 ]]; then + print_info_msg "\ + + Error: python version must be 3.6 or higher + python version: ${pyversion[*]}" + pyerrors=$((pyerrors+1)) + fi +fi + +#Next, check for the non-standard python packages: jinja2, yaml, and f90nml +pkgs=(jinja2 yaml f90nml) +for pkg in ${pkgs[@]} ; do + if ! /usr/bin/env python3 -c "import ${pkg}" &> /dev/null; then + print_info_msg "\ + + Error: python module ${pkg} not available" + pyerrors=$((pyerrors+1)) + fi +done + +#Finally, check if the number of errors is >0, and if so exit with helpful message +if [ $pyerrors -gt 0 ];then + print_err_msg_exit "\ + Errors found: check your python environment + + Instructions for setting up python environments can be found on the web: + https://github.com/ufs-community/ufs-srweather-app/wiki/Getting-Started + +" +fi +# +#----------------------------------------------------------------------- +# # Save current shell options (in a global array). Then set new options # for this script or function. # From 24211c1ba8d01bde6e1ff50ff6244671c4210ab9 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Thu, 13 Oct 2022 05:32:57 +0000 Subject: [PATCH 06/26] More updates, fixes, and cleanup - Fix problem with valid config option check - Convert calls to print_err_msg_exit() in ush/python_utils/config_parser.py - Designate MACHINE as a "mandatory" variable; it *must* be specified in config.yaml - Formatting cleanup with dedent --- ush/generate_FV3LAM_wflow.py | 9 +++++--- ush/python_utils/config_parser.py | 21 +++++++------------ ush/setup.py | 35 ++++++++++++++++++++----------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 894bf5b888..720db77139 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -1065,12 +1065,15 @@ def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: except: # If the call to the generate_FV3LAM_wflow function above was not successful, # print out an error message and exit with a nonzero return code. - logging.exception( + logging.exception(dedent( f""" + ********************************************************************* Experiment generation failed. See the error message(s) printed below. For more detailed information, check the log file from the workflow - generation script: {logfile}""" - ) + generation script: {logfile} + *********************************************************************\n + """ + )) class Testing(unittest.TestCase): def test_generate_FV3LAM_wflow(self): diff --git a/ush/python_utils/config_parser.py b/ush/python_utils/config_parser.py index b89ab2b5a7..7cae1b21f1 100644 --- a/ush/python_utils/config_parser.py +++ b/ush/python_utils/config_parser.py @@ -36,7 +36,6 @@ from xml.dom import minidom from .environment import list_to_str, str_to_list -from .print_msg import print_err_msg_exit from .run_command import run_command ########## @@ -49,7 +48,7 @@ def load_yaml_config(config_file): with open(config_file, "r") as f: cfg = yaml.safe_load(f) except yaml.YAMLError as e: - print_err_msg_exit(str(e)) + raise Exception(f"Unable to load yaml file {config_file}") return cfg @@ -102,7 +101,7 @@ def load_json_config(config_file): with open(config_file, "r") as f: cfg = json.load(f) except json.JSONDecodeError as e: - print_err_msg_exit(str(e)) + raise Exception(f"Unable to load json file {config_file}") return cfg @@ -218,11 +217,10 @@ def load_ini_config(config_file, return_string=0): """Load a config file with a format similar to Microsoft's INI files""" if not os.path.exists(config_file): - print_err_msg_exit( - f''' - The specified configuration file does not exist: - \"{config_file}\"''' - ) + raise FileNotFoundError(dedent(f''' + The specified configuration file does not exist: + "{config_file}"''' + )) config = configparser.RawConfigParser() config.optionxform = str @@ -238,12 +236,7 @@ def get_ini_value(config, section, key): """Finds the value of a property in a given section""" if not section in config: - print_err_msg_exit( - f''' - Section not found: - section = \"{section}\" - valid sections = \"{config.keys()}\"''' - ) + raise KeyError(f'Section not found: {section}') else: return config[section][key] diff --git a/ush/setup.py b/ush/setup.py index 0b8081cab5..cd670655c5 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -96,9 +96,17 @@ def setup(logger: Logger = getLogger()): cfg_u = load_config_file(os.path.join(USHdir, EXPT_CONFIG_FN)) cfg_u = flatten_dict(cfg_u) for key in cfg_u: - if key not in cfg_d: - raise Exception(dedent(f'''User-specified variable {key} in {EXPT_CONFIG_FN} is not valid - Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables\n''')) + if key not in flatten_dict(cfg_d): + raise Exception(dedent(f''' + User-specified variable "{key}" in {EXPT_CONFIG_FN} is not valid + Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables\n''')) + + # Mandatory variables *must* be set in the user's config; the default value is invalid + mandatory = ['MACHINE'] + for val in mandatory: + if val not in cfg_u: + raise Exception(f'Mandatory variable "{val}" not found in user config file {EXPT_CONFIG_FN}') + import_vars(dictionary=cfg_u, env_vars=["MACHINE", "EXTRN_MDL_NAME_ICS", "EXTRN_MDL_NAME_LBCS", "FV3GFS_FILE_FMT_ICS", "FV3GFS_FILE_FMT_LBCS"]) @@ -341,30 +349,33 @@ def get_location(xcs,fmt): mng_extrns_cfg_fn = os.readlink(mng_extrns_cfg_fn) except: pass - property_name = "local_path" cfg = load_ini_config(mng_extrns_cfg_fn) + # # Get the base directory of the FV3 forecast model code. # external_name = FCST_MODEL - UFS_WTHR_MDL_DIR = get_ini_value(cfg, external_name, property_name) + property_name = "local_path" - if not UFS_WTHR_MDL_DIR: - raise Exception( - f""" - Externals.cfg does not contain "{external_name}".""" - ) + try: + UFS_WTHR_MDL_DIR = get_ini_value(cfg, external_name, property_name) + except KeyError: + errmsg = dedent(f''' + Externals configuration file {mng_extrns_cfg_fn} + does not contain "{external_name}".''') + raise Exception(errmsg) from None + UFS_WTHR_MDL_DIR = os.path.join(HOMEdir, UFS_WTHR_MDL_DIR) if not os.path.exists(UFS_WTHR_MDL_DIR): - raise FileNotFoundError( + raise FileNotFoundError(dedent( f""" The base directory in which the FV3 source code should be located (UFS_WTHR_MDL_DIR) does not exist: UFS_WTHR_MDL_DIR = \"{UFS_WTHR_MDL_DIR}\" Please clone the external repository containing the code in this directory, build the executable, and then rerun the workflow.""" - ) + )) # # Define some other useful paths # From 2cad66b9e4166da60aa7f85c06a72ae666829020 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Thu, 13 Oct 2022 20:28:18 +0000 Subject: [PATCH 07/26] More error/logging cleanup in setup.py - Better error trap for config load failure - Explicitly raise error if machine file does not exist - Consolidate some more verbose comments - Print formatting improvements --- ush/setup.py | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/ush/setup.py b/ush/setup.py index cd670655c5..40adfeeccb 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -93,13 +93,21 @@ def setup(logger: Logger = getLogger()): if not os.path.exists(EXPT_CONFIG_FN): raise FileNotFoundError(f'User config file not found: {EXPT_CONFIG_FN=}') - cfg_u = load_config_file(os.path.join(USHdir, EXPT_CONFIG_FN)) + try: + cfg_u = load_config_file(os.path.join(USHdir, EXPT_CONFIG_FN)) + except: + errmsg = dedent(f'''\n + Could not load YAML config file: {EXPT_CONFIG_FN} + The file may be formatted incorrectly; reference the Users Guide for more info + ''') + raise Exception(errmsg) from None + cfg_u = flatten_dict(cfg_u) for key in cfg_u: if key not in flatten_dict(cfg_d): raise Exception(dedent(f''' - User-specified variable "{key}" in {EXPT_CONFIG_FN} is not valid - Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables\n''')) + User-specified variable "{key}" in {EXPT_CONFIG_FN} is not valid + Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables\n''')) # Mandatory variables *must* be set in the user's config; the default value is invalid mandatory = ['MACHINE'] @@ -121,6 +129,12 @@ def setup(logger: Logger = getLogger()): # global MACHINE, EXTRN_MDL_SYSBASEDIR_ICS, EXTRN_MDL_SYSBASEDIR_LBCS MACHINE_FILE = os.path.join(USHdir, "machine", f"{lowercase(MACHINE)}.yaml") + if not os.path.exists(MACHINE_FILE): + raise FileNotFoundError(dedent( + f""" + The machine file {MACHINE_FILE} does not exist. + Check that you have specified the correct machine ({MACHINE=}) in your config file {EXPT_CONFIG_FN}""" + )) machine_cfg = load_config_file(MACHINE_FILE) # ics and lbcs @@ -165,20 +179,14 @@ def get_location(xcs,fmt): # make machine name uppercase MACHINE = uppercase(MACHINE) - # - # ----------------------------------------------------------------------- - # - # Source constants.sh and save its contents to a variable for later - # - # ----------------------------------------------------------------------- - # + # Load constants file and save its contents to a variable for later cfg_c = load_config_file(os.path.join(USHdir, CONSTANTS_FN)) import_vars(dictionary=flatten_dict(cfg_c)) # # ----------------------------------------------------------------------- # - # Generate a unique number for this workflow run. This maybe used to + # Generate a unique number for this workflow run. This may be used to # get unique log file names for example # # ----------------------------------------------------------------------- @@ -1984,8 +1992,8 @@ def get_location(xcs,fmt): if (vkey in cfg_v) and not (v in cfg_v[vkey]): raise Exception( f""" - The variable {k}={v} in {EXPT_DEFAULT_CONFIG_FN} or {EXPT_CONFIG_FN} does not have - a valid value. Possible values are: + The variable {k}={v} in {EXPT_DEFAULT_CONFIG_FN} or {EXPT_CONFIG_FN} + does not have a valid value. Possible values are: {k} = {cfg_v[vkey]}""" ) From 6e22e7d60df30f80f04c13518163dec7baaf2efa Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Thu, 13 Oct 2022 22:07:52 +0000 Subject: [PATCH 08/26] More error/logging cleanup in setup.py and other files - Change config_defaults value for ACCOUNT to empty string, so the logic to catch unset ACCOUNT will trigger properly - Also fix existing error message - Trap errors for incorrect PREDEF_GRID_NAME setting - Fix error messages for various SPP settings - Don't re-set USHdir - Consolidate and python-ize the check for missing machine file settings - Consolidate more verbose comments --- ush/config_defaults.yaml | 2 +- ush/set_predef_grid_params.py | 10 ++++- ush/setup.py | 69 ++++++++++++++++------------------- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/ush/config_defaults.yaml b/ush/config_defaults.yaml index 96bca2e66f..2c5831a4df 100644 --- a/ush/config_defaults.yaml +++ b/ush/config_defaults.yaml @@ -54,7 +54,7 @@ user: # #----------------------------------------------------------------------- MACHINE: "BIG_COMPUTER" - ACCOUNT: "project_name" + ACCOUNT: "" #---------------------------- # PLATFORM config parameters diff --git a/ush/set_predef_grid_params.py b/ush/set_predef_grid_params.py index 2e48e5b4a8..226d17065c 100644 --- a/ush/set_predef_grid_params.py +++ b/ush/set_predef_grid_params.py @@ -2,6 +2,7 @@ import unittest import os +from textwrap import dedent from python_utils import ( import_vars, @@ -37,7 +38,14 @@ def set_predef_grid_params(): USHdir = os.path.dirname(os.path.abspath(__file__)) params_dict = load_config_file(os.path.join(USHdir, "predef_grid_params.yaml")) - params_dict = params_dict[PREDEF_GRID_NAME] + try: + params_dict = params_dict[PREDEF_GRID_NAME] + except KeyError: + errmsg = dedent(f''' + {PREDEF_GRID_NAME=} not found in predef_grid_params.yaml + Check your config file settings.''') + raise Exception(errmsg) from None + # if QUILTING = False, remove key if not QUILTING: diff --git a/ush/setup.py b/ush/setup.py index 40adfeeccb..3d6e9c931a 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -304,9 +304,17 @@ def get_location(xcs,fmt): raise Exception( f''' All MYNN PBL, MYNN SFC, GSL GWD, Thompson MP, or RRTMG SPP-related namelist - variables set in {CONFIG_FN} must be equal in number of entries to what is + variables set in {EXPT_CONFIG_FN} must be equal in number of entries to what is found in SPP_VAR_LIST: - Number of entries in SPP_VAR_LIST = \"{len(SPP_VAR_LIST)}\"''' + {len(SPP_VAR_LIST)=} + {len(SPP_MAG_LIST)=} + {len(SPP_LSCALE)=} + {len(SPP_TSCALE)=} + {len(SPP_SIGTOP1)=} + {len(SPP_SIGTOP2)=} + {len(SPP_STDDEV_CUTOFF)=} + {len(ISEED_SPP)=} + ''' ) # # ----------------------------------------------------------------------- @@ -326,9 +334,13 @@ def get_location(xcs,fmt): raise Exception( f''' All Noah or RUC-LSM SPP-related namelist variables (except ISEED_LSM_SPP) - set in {CONFIG_FN} must be equal in number of entries to what is found in + set in {EXPT_CONFIG_FN} must be equal in number of entries to what is found in SPP_VAR_LIST: - Number of entries in SPP_VAR_LIST = \"{len(LSM_SPP_VAR_LIST)}\"''' + {len(LSM_SPP_VAR_LIST)=} + {len(LSM_SPP_MAG_LIST)=} + {len(LSM_SPP_LSCALE)=} + {len(LSM_SPP_TSCALE)=} + ''' ) # # The current script should be located in the ush subdirectory of the @@ -390,7 +402,6 @@ def get_location(xcs,fmt): global SCRIPTSdir, JOBSdir, SORCdir, PARMdir, MODULESdir global EXECdir, PARMdir, FIXdir, VX_CONFIG_DIR, METPLUS_CONF, MET_CONFIG - USHdir = os.path.join(HOMEdir, "ush") SCRIPTSdir = os.path.join(HOMEdir, "scripts") JOBSdir = os.path.join(HOMEdir, "jobs") SORCdir = os.path.join(HOMEdir, "sorc") @@ -415,26 +426,17 @@ def get_location(xcs,fmt): RELATIVE_LINK_FLAG = "--relative" - if not NCORES_PER_NODE: - raise Exception( - f""" - NCORES_PER_NODE has not been specified in the file {MACHINE_FILE} - Please ensure this value has been set for your desired platform. """ - ) - - if not (FIXgsm and FIXaer and FIXlut and TOPO_DIR and SFC_CLIMO_INPUT_DIR): - raise Exception( - f""" - One or more fix file directories have not been specified for this machine: - MACHINE = \"{MACHINE}\" - FIXgsm = \"{FIXgsm or ""} - FIXaer = \"{FIXaer or ""} - FIXlut = \"{FIXlut or ""} - TOPO_DIR = \"{TOPO_DIR or ""} - SFC_CLIMO_INPUT_DIR = \"{SFC_CLIMO_INPUT_DIR or ""} - DOMAIN_PREGEN_BASEDIR = \"{DOMAIN_PREGEN_BASEDIR or ""} - You can specify the missing location(s) in config.sh""" - ) + # Mandatory variables *must* be set in the user's config or the machine file; the default value is invalid + mandatory = ['NCORES_PER_NODE', 'FIXgsm', 'FIXaer', 'FIXlut', 'TOPO_DIR', 'SFC_CLIMO_INPUT_DIR'] + for val in mandatory: + # globals() returns dictionary of global variables + if not globals()[val]: + raise Exception(dedent(f''' + Mandatory variable "{val}" not found in: + user config file {EXPT_CONFIG_FN} + OR + machine file {MACHINE_FILE} + ''')) # # ----------------------------------------------------------------------- @@ -474,12 +476,10 @@ def get_location(xcs,fmt): # if WORKFLOW_MANAGER is not None: if not ACCOUNT: - raise Exception( - f''' - The variable ACCOUNT cannot be empty if you are using a workflow manager: - ACCOUNT = \"ACCOUNT\" - WORKFLOW_MANAGER = \"WORKFLOW_MANAGER\"''' - ) + raise Exception(dedent(f''' + ACCOUNT must be specified in config or machine file if using a workflow manager. + {WORKFLOW_MANAGER=}\n''' + )) # # ----------------------------------------------------------------------- # @@ -496,12 +496,7 @@ def get_location(xcs,fmt): GTYPE = "regional" TILE_RGNL = "7" - # ----------------------------------------------------------------------- - # - # Set USE_MERRA_CLIMO to either "TRUE" or "FALSE" so we don't - # have to consider other valid values later on. - # - # ----------------------------------------------------------------------- + # USE_MERRA_CLIMO must be True for the physics suite FV3_GFS_v15_thompson_mynn_lam3km" global USE_MERRA_CLIMO if CCPP_PHYS_SUITE == "FV3_GFS_v15_thompson_mynn_lam3km": USE_MERRA_CLIMO = True From f749d159305bf4d1f21d34d780cc7a2c50041117 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Fri, 14 Oct 2022 18:20:02 +0000 Subject: [PATCH 09/26] More error/logging cleanup in setup.py - Consolidate several variable checks into loops over a list of variable names - Date variables are now a bit more flexible in format; change error message to match - Update and consolidate more comments --- ush/setup.py | 146 ++++++++++++++++----------------------------------- 1 file changed, 46 insertions(+), 100 deletions(-) diff --git a/ush/setup.py b/ush/setup.py index 3d6e9c931a..9f24fa9aae 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -518,46 +518,25 @@ def get_location(xcs,fmt): The coupling flag CPL has not been specified for this value of FCST_MODEL: FCST_MODEL = \"{FCST_MODEL}\"''' ) - # - # ----------------------------------------------------------------------- - # - # Make sure RESTART_INTERVAL is set to an integer value if present - # - # ----------------------------------------------------------------------- - # + + # Make sure RESTART_INTERVAL is set to an integer value if not isinstance(RESTART_INTERVAL, int): - raise Exception( - f''' - RESTART_INTERVAL must be set to an integer number of hours. - RESTART_INTERVAL = \"{RESTART_INTERVAL}\"''' - ) - # - # ----------------------------------------------------------------------- - # - # Check that DATE_FIRST_CYCL and DATE_LAST_CYCL are strings consisting - # of exactly 10 digits. - # - # ----------------------------------------------------------------------- - # - if not isinstance(DATE_FIRST_CYCL, datetime.date): - raise Exception( - f''' - DATE_FIRST_CYCL must be a string consisting of exactly 10 digits of the - form \"YYYYMMDDHH\", where YYYY is the 4-digit year, MM is the 2-digit - month, DD is the 2-digit day-of-month, and HH is the 2-digit - cycle hour. - DATE_FIRST_CYCL = \"{DATE_FIRST_CYCL}\"''' - ) + raise Exception(f"\n{RESTART_INTERVAL=}, must be an integer value\n") + + # Check that input dates are in a date format + + # get dictionary of all variables + allvars = dict(globals()) + allvars.update(locals()) + dates = ['DATE_FIRST_CYCL', 'DATE_LAST_CYCL'] + for val in dates: + if not isinstance(allvars[val], datetime.date): + raise Exception(dedent(f''' + Date variable {val}={allvars[val]} is not in a valid date format + + For examples of valid formats, see the users guide. + ''')) - if not isinstance(DATE_LAST_CYCL, datetime.date): - raise Exception( - f''' - DATE_LAST_CYCL must be a string consisting of exactly 10 digits of the - form \"YYYYMMDDHH\", where YYYY is the 4-digit year, MM is the 2-digit - month, DD is the 2-digit day-of-month, and HH is the 2-digit - cycle hour. - DATE_LAST_CYCL = \"{DATE_LAST_CYCL}\"''' - ) # # ----------------------------------------------------------------------- # @@ -589,23 +568,30 @@ def get_location(xcs,fmt): # If using a custom post configuration file, make sure that it exists. if USE_CUSTOM_POST_CONFIG_FILE: - if not os.path.exists(CUSTOM_POST_CONFIG_FP): - raise FileNotFoundError( + try: + if not os.path.exists(CUSTOM_POST_CONFIG_FP): + raise + except: + raise FileNotFoundError(dedent( f''' - The custom post configuration specified by CUSTOM_POST_CONFIG_FP does not - exist: - CUSTOM_POST_CONFIG_FP = \"{CUSTOM_POST_CONFIG_FP}\"''' - ) + USE_CUSTOM_POST_CONFIG_FILE has been set, but the custom post configuration file + {CUSTOM_POST_CONFIG_FP=} + could not be found.''' + )) from None # If using external CRTM fix files to allow post-processing of synthetic # satellite products from the UPP, make sure the CRTM fix file directory exists. if USE_CRTM: - if not os.path.exists(CRTM_DIR): - raise FileNotFoundError( + try: + if not os.path.exists(CRTM_DIR): + raise + except: + raise FileNotFoundError(dedent( f''' - The external CRTM fix file directory specified by CRTM_DIR does not exist: - CRTM_DIR = \"{CRTM_DIR}\"''' - ) + USE_CRTM has been set, but the external CRTM fix file directory: + {CRTM_DIR=} + could not be found.''' + )) from None # The forecast length (in integer hours) cannot contain more than 3 characters. # Thus, its maximum value is 999. @@ -663,48 +649,18 @@ def get_location(xcs,fmt): # # ----------------------------------------------------------------------- # - if not DT_ATMOS: - raise Exception( - f''' - The forecast model main time step (DT_ATMOS) is set to a null string: - DT_ATMOS = {DT_ATMOS} - Please set this to a valid numerical value in the user-specified experiment - configuration file (EXPT_CONFIG_FP) and rerun: - EXPT_CONFIG_FP = \"{EXPT_CONFIG_FP}\"''' - ) - - if not LAYOUT_X: - raise Exception( - f''' - The number of MPI processes to be used in the x direction (LAYOUT_X) by - the forecast job is set to a null string: - LAYOUT_X = {LAYOUT_X} - Please set this to a valid numerical value in the user-specified experiment - configuration file (EXPT_CONFIG_FP) and rerun: - EXPT_CONFIG_FP = \"{EXPT_CONFIG_FP}\"''' - ) + # get dictionary of all variables + allvars = dict(globals()) + allvars.update(locals()) + vlist = ['DT_ATMOS', + 'LAYOUT_X', + 'LAYOUT_Y', + 'BLOCKSIZE', + 'EXPT_SUBDIR'] + for val in vlist: + if not allvars[val]: + raise Exception(f"\nMandatory variable '{val}' has not been set\n") - if not LAYOUT_Y: - raise Exception( - f''' - The number of MPI processes to be used in the y direction (LAYOUT_Y) by - the forecast job is set to a null string: - LAYOUT_Y = {LAYOUT_Y} - Please set this to a valid numerical value in the user-specified experiment - configuration file (EXPT_CONFIG_FP) and rerun: - EXPT_CONFIG_FP = \"{EXPT_CONFIG_FP}\"''' - ) - - if not BLOCKSIZE: - raise Exception( - f''' - The cache size to use for each MPI task of the forecast (BLOCKSIZE) is - set to a null string: - BLOCKSIZE = {BLOCKSIZE} - Please set this to a valid numerical value in the user-specified experiment - configuration file (EXPT_CONFIG_FP) and rerun: - EXPT_CONFIG_FP = \"{EXPT_CONFIG_FP}\"''' - ) # # ----------------------------------------------------------------------- # @@ -796,13 +752,6 @@ def get_location(xcs,fmt): mkdir_vrfy(f' -p "{EXPT_BASEDIR}"') - # The experiment subdirectory name (EXPT_SUBDIR) can not be set to an empty string - if not EXPT_SUBDIR: - raise Exception( - f''' - The name of the experiment subdirectory (EXPT_SUBDIR) cannot be empty: - EXPT_SUBDIR = \"{EXPT_SUBDIR}\"''' - ) # # ----------------------------------------------------------------------- # @@ -951,9 +900,6 @@ def get_location(xcs,fmt): # (4) The FV3 namelist file # (5) The model configuration file # (6) The NEMS configuration file - # - # If using CCPP, it also needs: - # # (7) The CCPP physics suite definition file # # The workflow contains templates for the first six of these files. From c3cc0d3bff4f7ba126162691f833611317dda23c Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Fri, 14 Oct 2022 22:46:06 +0000 Subject: [PATCH 10/26] Better logging name strategy, more error/logging cleanup in setup.py and other files - No longer pass new logging object to subroutines, simply allow functions to inherit logging and rename logging within the function - Eliminate print_info_msg and print_err_msg_exit from set_ozone_param.py - Remove unused and unneeded ALL_CDATES and NUM_CYCLES variables - Consolidate/eliminate more overly verbose comments - Change logic slightly for when RUN_TASK_MAKE_GRID, RUN_TASK_MAKE_OROG, or RUN_TASK_MAKE_SFC_CLIMO is false: First check if the fix dir is not specified and change to default location (with warning message), THEN check to see if the dir exists (and fail if it does not) --- ush/generate_FV3LAM_wflow.py | 2 +- ush/set_ozone_param.py | 14 ++- ush/setup.py | 161 ++++++++++++----------------------- 3 files changed, 60 insertions(+), 117 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 720db77139..5c041ff836 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -101,7 +101,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # The setup function reads the user configuration file and fills in # non-user-specified values from config_defaults.yaml - setup(logging.getLogger('root.setup')) + setup() # import all environment variables import_vars() diff --git a/ush/set_ozone_param.py b/ush/set_ozone_param.py index e371791452..7a7a4ae75c 100644 --- a/ush/set_ozone_param.py +++ b/ush/set_ozone_param.py @@ -3,6 +3,7 @@ import os import unittest from textwrap import dedent +from logging import getLogger from python_utils import ( import_vars, @@ -10,15 +11,12 @@ set_env_var, list_to_str, print_input_args, - print_info_msg, - print_err_msg_exit, define_macos_utilities, load_xml_file, has_tag_with_value, find_pattern_in_str, ) - def set_ozone_param(ccpp_phys_suite_fp): """Function that does the following: (1) Determines the ozone parameterization being used by checking in the @@ -47,6 +45,7 @@ def set_ozone_param(ccpp_phys_suite_fp): ozone_param: a string """ + logger = getLogger(__name__) print_input_args(locals()) # import all environment variables @@ -92,12 +91,11 @@ def set_ozone_param(ccpp_phys_suite_fp): fixgsm_ozone_fn = "global_o3prdlos.f77" ozone_param = "ozphys" else: - print_err_msg_exit( + raise Exception( f''' Unknown or no ozone parameterization specified in the CCPP physics suite file (ccpp_phys_suite_fp): - ccpp_phys_suite_fp = \"{ccpp_phys_suite_fp}\" - ozone_param = \"{ozone_param}\"''' + ccpp_phys_suite_fp = \"{ccpp_phys_suite_fp}\"''' ) # # ----------------------------------------------------------------------- @@ -167,11 +165,11 @@ def set_ozone_param(ccpp_phys_suite_fp): CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING = {list_to_str(CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING)} """ ) - print_info_msg(msg, verbose=VERBOSE) + logger.info(msg) else: - print_err_msg_exit( + raise Exception( f''' Unable to set name of the ozone production/loss file in the FIXgsm directory in the array that specifies the mapping between the symlinks that need to diff --git a/ush/setup.py b/ush/setup.py index 9f24fa9aae..3f09382368 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -4,7 +4,7 @@ import sys import datetime from textwrap import dedent -from logging import Logger, getLogger +from logging import getLogger from python_utils import ( cd_vrfy, @@ -38,7 +38,7 @@ from set_thompson_mp_fix_files import set_thompson_mp_fix_files -def setup(logger: Logger = getLogger()): +def setup(): """Function that sets a secondary set of parameters needed by the various scripts that are called by the FV3-LAM rocoto community workflow. This secondary set of parameters is @@ -50,12 +50,12 @@ def setup(logger: Logger = getLogger()): scripts called by the tasks in the workflow. Args: - logger: A logger object for Python's built in logging module. Typically - this is set up in generate_FV3LAM_workflow and passed here. + None Returns: None """ + logger = getLogger(__name__) global USHdir USHdir = os.path.dirname(os.path.abspath(__file__)) cd_vrfy(USHdir) @@ -220,7 +220,7 @@ def get_location(xcs,fmt): # global VERBOSE if DEBUG and not VERBOSE: - logging.info( + logger.info( """ Resetting VERBOSE to \"TRUE\" because DEBUG has been set to \"TRUE\"...""" ) @@ -537,35 +537,6 @@ def get_location(xcs,fmt): For examples of valid formats, see the users guide. ''')) - # - # ----------------------------------------------------------------------- - # - # Call a function to generate the array ALL_CDATES containing the cycle - # dates/hours for which to run forecasts. The elements of this array - # will have the form YYYYMMDDHH. They are the starting dates/times of - # the forecasts that will be run in the experiment. Then set NUM_CYCLES - # to the number of elements in this array. - # - # ----------------------------------------------------------------------- - # - - ALL_CDATES = set_cycle_dates( - date_start=DATE_FIRST_CYCL, - date_end=DATE_LAST_CYCL, - incr_cycl_freq=INCR_CYCL_FREQ, - ) - - NUM_CYCLES = len(ALL_CDATES) - - # Completely arbitrary cutoff of 90 cycles. - if NUM_CYCLES > 90: - ALL_CDATES = None - logger.warning( - f""" - Too many cycles in ALL_CDATES to list, redefining in abbreviated form." - ALL_CDATES="{DATE_FIRST_CYCL}...{DATE_LAST_CYCL}""" - ) - # If using a custom post configuration file, make sure that it exists. if USE_CUSTOM_POST_CONFIG_FILE: try: @@ -1032,7 +1003,7 @@ def get_location(xcs,fmt): # export env vars before calling another module export_vars() - OZONE_PARAM = set_ozone_param(ccpp_phys_suite_fp=CCPP_PHYS_SUITE_IN_CCPP_FP) + OZONE_PARAM = set_ozone_param(CCPP_PHYS_SUITE_IN_CCPP_FP) IMPORTS = ["CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING", "FIXgsm_FILES_TO_COPY_TO_FIXam"] import_vars(env_vars=IMPORTS) @@ -1111,10 +1082,9 @@ def get_location(xcs,fmt): # # ----------------------------------------------------------------------- # - # Make sure that DO_ENSEMBLE is set to a valid value. Then set the names - # of the ensemble members. These will be used to set the ensemble member - # directories. Also, set the full path to the FV3 namelist file corresponding - # to each ensemble member. + # If DO_ENSEMBLE, set the names of the ensemble members; these will be + # used to set the ensemble member directories. Also, set the full path + # to the FV3 namelist file corresponding to each ensemble member. # # ----------------------------------------------------------------------- # @@ -1130,13 +1100,8 @@ def get_location(xcs,fmt): FV3_NML_ENSMEM_FPS.append( os.path.join(EXPTDIR, f"{FV3_NML_FN}_{ENSMEM_NAMES[i]}") ) - # - # ----------------------------------------------------------------------- - # + # Set the full path to the forecast model executable. - # - # ----------------------------------------------------------------------- - # global FV3_EXEC_FP FV3_EXEC_FP = os.path.join(EXECdir, FV3_EXEC_FN) # @@ -1171,13 +1136,6 @@ def get_location(xcs,fmt): global LOAD_MODULES_RUN_TASK_FP LOAD_MODULES_RUN_TASK_FP = os.path.join(USHdir, "load_modules_run_task.sh") - # - # ----------------------------------------------------------------------- - # - # Turn off some tasks that can not be run in NCO mode - # - # ----------------------------------------------------------------------- - # global RUN_TASK_MAKE_GRID, RUN_TASK_MAKE_OROG, RUN_TASK_MAKE_SFC_CLIMO global RUN_TASK_VX_GRIDSTAT, RUN_TASK_VX_POINTSTAT, RUN_TASK_VX_ENSGRID, RUN_TASK_VX_ENSPOINT @@ -1191,13 +1149,7 @@ def get_location(xcs,fmt): FIXclim = os.path.join(FIXdir, "fix_clim") FIXlam = os.path.join(FIXdir, "fix_lam") - # - # ----------------------------------------------------------------------- - # - # Make sure that DO_ENSEMBLE is set to TRUE when running ensemble vx. - # - # ----------------------------------------------------------------------- - # + # Ensemble verification can only be run in ensemble mode if (not DO_ENSEMBLE) and (RUN_TASK_VX_ENSGRID or RUN_TASK_VX_ENSPOINT): raise Exception( f''' @@ -1245,21 +1197,22 @@ def get_location(xcs,fmt): # experiment directory (EXPTDIR). # if not RUN_TASK_MAKE_GRID: - if (GRID_DIR is None) or (not os.path.exists(GRID_DIR)): + if (GRID_DIR is None): GRID_DIR = os.path.join(DOMAIN_PREGEN_BASEDIR, PREDEF_GRID_NAME) - msg = f"""Setting GRID_DIR to: - GRID_DIR = \"{GRID_DIR}\" - """ - logger.info(msg) - - if not os.path.exists(GRID_DIR): - raise FileNotFoundError( - f''' - The directory (GRID_DIR) that should contain the pregenerated grid files - does not exist: - GRID_DIR = \"{GRID_DIR}\"''' - ) + msg = dedent(f""" + GRID_DIR not specified! + Setting {GRID_DIR=} + """) + logger.warning(msg) + + if not os.path.exists(GRID_DIR): + raise FileNotFoundError( + f''' + The directory (GRID_DIR) that should contain the pregenerated grid files + does not exist: + GRID_DIR = \"{GRID_DIR}\"''' + ) else: GRID_DIR = os.path.join(EXPTDIR, "grid") # @@ -1269,21 +1222,22 @@ def get_location(xcs,fmt): # the experiment directory (EXPTDIR). # if not RUN_TASK_MAKE_OROG: - if (OROG_DIR is None) or (not os.path.exists(OROG_DIR)): + if (OROG_DIR is None): OROG_DIR = os.path.join(DOMAIN_PREGEN_BASEDIR, PREDEF_GRID_NAME) - msg = f"""Setting OROG_DIR to: - OROG_DIR = \"{OROG_DIR}\" - """ - logger.info(msg) - - if not os.path.exists(OROG_DIR): - raise FileNotFoundError( - f''' - The directory (OROG_DIR) that should contain the pregenerated orography - files does not exist: - OROG_DIR = \"{OROG_DIR}\"''' - ) + msg = dedent(f""" + OROG_DIR not specified! + Setting {OROG_DIR=} + """) + logger.warning(msg) + + if not os.path.exists(OROG_DIR): + raise FileNotFoundError( + f''' + The directory (OROG_DIR) that should contain the pregenerated orography + files does not exist: + OROG_DIR = \"{OROG_DIR}\"''' + ) else: OROG_DIR = os.path.join(EXPTDIR, "orog") # @@ -1293,21 +1247,22 @@ def get_location(xcs,fmt): # a predefined location under the experiment directory (EXPTDIR). # if not RUN_TASK_MAKE_SFC_CLIMO: - if (SFC_CLIMO_DIR is None) or (not os.path.exists(SFC_CLIMO_DIR)): + if (SFC_CLIMO_DIR is None): SFC_CLIMO_DIR = os.path.join(DOMAIN_PREGEN_BASEDIR, PREDEF_GRID_NAME) - msg = f"""Setting SFC_CLIMO_DIR to: - SFC_CLIMO_DIR = \"{SFC_CLIMO_DIR}\" - """ - logger.info(msg) - - if not os.path.exists(SFC_CLIMO_DIR): - raise FileNotFoundError( - f''' - The directory (SFC_CLIMO_DIR) that should contain the pregenerated surface - climatology files does not exist: - SFC_CLIMO_DIR = \"{SFC_CLIMO_DIR}\"''' - ) + msg = dedent(f""" + SFC_CLIMO_DIR not specified! + Setting {SFC_CLIMO_DIR=} + """) + logger.warning(msg) + + if not os.path.exists(SFC_CLIMO_DIR): + raise FileNotFoundError( + f''' + The directory (SFC_CLIMO_DIR) that should contain the pregenerated surface + climatology files does not exist: + SFC_CLIMO_DIR = \"{SFC_CLIMO_DIR}\"''' + ) else: SFC_CLIMO_DIR = os.path.join(EXPTDIR, "sfc_climo") @@ -1819,16 +1774,6 @@ def get_location(xcs,fmt): # # ----------------------------------------------------------------------- # - # The number of cycles for which to make forecasts and the list of - # starting dates/hours of these cycles. - # - # ----------------------------------------------------------------------- - # - "NUM_CYCLES": NUM_CYCLES, - "ALL_CDATES": ALL_CDATES, - # - # ----------------------------------------------------------------------- - # # Computational parameters. # # ----------------------------------------------------------------------- From 0535f8d7ee2b81d6d46809ac2dcd5d65d312ff06 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Sat, 15 Oct 2022 03:40:24 +0000 Subject: [PATCH 11/26] Finish error/logging cleanup in setup.py, set_thompson_mp_fix_files.py - Consolidate more comments - Add warning for inline post turning off post-processing tasks - Consolidate unnecessary splitting of "settings" array --- ush/set_thompson_mp_fix_files.py | 6 +- ush/setup.py | 122 ++++++++++++++----------------- 2 files changed, 57 insertions(+), 71 deletions(-) diff --git a/ush/set_thompson_mp_fix_files.py b/ush/set_thompson_mp_fix_files.py index 0d7a454ad4..76f230d628 100644 --- a/ush/set_thompson_mp_fix_files.py +++ b/ush/set_thompson_mp_fix_files.py @@ -3,6 +3,7 @@ import os import unittest from textwrap import dedent +from logging import getLogger from python_utils import ( import_vars, @@ -10,8 +11,6 @@ set_env_var, list_to_str, print_input_args, - print_info_msg, - print_err_msg_exit, define_macos_utilities, load_xml_file, has_tag_with_value, @@ -36,6 +35,7 @@ def set_thompson_mp_fix_files(ccpp_phys_suite_fp, thompson_mp_climo_fn): boolean: sdf_uses_thompson_mp """ + logger = getLogger(__name__) print_input_args(locals()) # import all environment variables @@ -114,7 +114,7 @@ def set_thompson_mp_fix_files(ccpp_phys_suite_fp, thompson_mp_climo_fn): CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING = {list_to_str(CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING)} """ ) - print_info_msg(msg) + logger.info(msg) EXPORTS = [ "CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING", diff --git a/ush/setup.py b/ush/setup.py index 3f09382368..02f16023b1 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -1348,10 +1348,9 @@ def get_location(xcs,fmt): # # ----------------------------------------------------------------------- # - # Create a new experiment directory. Note that at this point we are - # guaranteed that there is no preexisting experiment directory. For - # platforms with no workflow manager, we need to create LOGDIR as well, - # since it won't be created later at runtime. + # Create a new experiment directory. For platforms with no workflow + # manager we need to create LOGDIR as well, since it won't be created + # later at runtime. # # ----------------------------------------------------------------------- # @@ -1359,7 +1358,7 @@ def get_location(xcs,fmt): mkdir_vrfy(f' -p "{LOGDIR}"') # # ----------------------------------------------------------------------- - # + # NOTE: currently this is executed no matter what, should it be dependent on the logic described below?? # If not running the MAKE_GRID_TN, MAKE_OROG_TN, and/or MAKE_SFC_CLIMO # tasks, create symlinks under the FIXlam directory to pregenerated grid, # orography, and surface climatology files. In the process, also set @@ -1455,17 +1454,16 @@ def get_location(xcs,fmt): CRES = "" if not RUN_TASK_MAKE_GRID: CRES = f"C{RES_IN_FIXLAM_FILENAMES}" - # - # ----------------------------------------------------------------------- - # - # Make sure that WRITE_DOPOST is set to a valid value. - # - # ----------------------------------------------------------------------- - # + global RUN_TASK_RUN_POST if WRITE_DOPOST: # Turn off run_post - RUN_TASK_RUN_POST = False + if RUN_TASK_RUN_POST: + logger.warning(dedent(f""" + Inline post is turned on, deactivating post-processing tasks: + RUN_TASK_RUN_POST = False + """)) + RUN_TASK_RUN_POST = False # Check if SUB_HOURLY_POST is on if SUB_HOURLY_POST: @@ -1742,62 +1740,50 @@ def get_location(xcs,fmt): # the make_grid task is complete. # "CRES": CRES, + # + # ----------------------------------------------------------------------- + # + # Flag in the \"{MODEL_CONFIG_FN}\" file for coupling the ocean model to + # the weather model. + # + # ----------------------------------------------------------------------- + # + "CPL": CPL, + # + # ----------------------------------------------------------------------- + # + # Name of the ozone parameterization. The value this gets set to depends + # on the CCPP physics suite being used. + # + # ----------------------------------------------------------------------- + # + "OZONE_PARAM": OZONE_PARAM, + # + # ----------------------------------------------------------------------- + # + # Computational parameters. + # + # ----------------------------------------------------------------------- + # + "PE_MEMBER01": PE_MEMBER01, + # + # ----------------------------------------------------------------------- + # + # IF DO_SPP is set to "TRUE", N_VAR_SPP specifies the number of physics + # parameterizations that are perturbed with SPP. If DO_LSM_SPP is set to + # "TRUE", N_VAR_LNDP specifies the number of LSM parameters that are + # perturbed. LNDP_TYPE determines the way LSM perturbations are employed + # and FHCYC_LSM_SPP_OR_NOT sets FHCYC based on whether LSM perturbations + # are turned on or not. + # + # ----------------------------------------------------------------------- + # + "N_VAR_SPP": N_VAR_SPP, + "N_VAR_LNDP": N_VAR_LNDP, + "LNDP_TYPE": LNDP_TYPE, + "LNDP_MODEL_TYPE": LNDP_MODEL_TYPE, + "FHCYC_LSM_SPP_OR_NOT": FHCYC_LSM_SPP_OR_NOT, } - # - # ----------------------------------------------------------------------- - # - # Continue appending variable definitions to the variable definitions - # file. - # - # ----------------------------------------------------------------------- - # - settings.update( - { - # - # ----------------------------------------------------------------------- - # - # Flag in the \"{MODEL_CONFIG_FN}\" file for coupling the ocean model to - # the weather model. - # - # ----------------------------------------------------------------------- - # - "CPL": CPL, - # - # ----------------------------------------------------------------------- - # - # Name of the ozone parameterization. The value this gets set to depends - # on the CCPP physics suite being used. - # - # ----------------------------------------------------------------------- - # - "OZONE_PARAM": OZONE_PARAM, - # - # ----------------------------------------------------------------------- - # - # Computational parameters. - # - # ----------------------------------------------------------------------- - # - "PE_MEMBER01": PE_MEMBER01, - # - # ----------------------------------------------------------------------- - # - # IF DO_SPP is set to "TRUE", N_VAR_SPP specifies the number of physics - # parameterizations that are perturbed with SPP. If DO_LSM_SPP is set to - # "TRUE", N_VAR_LNDP specifies the number of LSM parameters that are - # perturbed. LNDP_TYPE determines the way LSM perturbations are employed - # and FHCYC_LSM_SPP_OR_NOT sets FHCYC based on whether LSM perturbations - # are turned on or not. - # - # ----------------------------------------------------------------------- - # - "N_VAR_SPP": N_VAR_SPP, - "N_VAR_LNDP": N_VAR_LNDP, - "LNDP_TYPE": LNDP_TYPE, - "LNDP_MODEL_TYPE": LNDP_MODEL_TYPE, - "FHCYC_LSM_SPP_OR_NOT": FHCYC_LSM_SPP_OR_NOT, - } - ) # write derived settings cfg_d["derived"] = settings From f0507fb8d848d1d860a847d8c4c8f4b529eab9ad Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Sat, 15 Oct 2022 04:29:22 +0000 Subject: [PATCH 12/26] Last set of initial changes! Now just need to test and review... - Remove/consolidate outdated or unnecessary comments - Remove print of rocoto details; it is now impossible for users to get this far *without* having loaded a module that includes rocoto - Add logging to get_crontab_contents.py --- ush/generate_FV3LAM_wflow.py | 38 ++----------------- ush/get_crontab_contents.py | 20 +++++----- .../check_for_preexist_dir_file.py | 11 +++--- ush/setup.py | 13 ++++++- 4 files changed, 31 insertions(+), 51 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 5c041ff836..0139776bd7 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -123,8 +123,8 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # Create a multiline variable that consists of a yaml-compliant string # specifying the values that the jinja variables in the template rocoto # XML should be set to. These values are set either in the user-specified - # workflow configuration file (EXPT_CONFIG_FN) or in the setup.sh script - # sourced above. Then call the python script that generates the XML. + # workflow configuration file (EXPT_CONFIG_FN) or in the setup() function + # called above. Then call the python script that generates the XML. # # ----------------------------------------------------------------------- # @@ -492,19 +492,9 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # if USE_CRON_TO_RELAUNCH: add_crontab_line() - # - # ----------------------------------------------------------------------- - # - # Create the FIXam directory under the experiment directory. In NCO mode, - # this will be a symlink to the directory specified in FIXgsm, while in - # community mode, it will be an actual directory with files copied into - # it from FIXgsm. - # - # ----------------------------------------------------------------------- - # # - # Symlink fix files + # Copy or symlink fix files # if SYMLINK_FIX_FILES: @@ -516,9 +506,6 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> ) ln_vrfy(f'''-fsn "{FIXgsm}" "{FIXam}"''') - # - # Copy relevant fix files. - # else: logging.info( @@ -638,9 +625,6 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # Otherwise, leave it unspecified (which means it gets set to the default # value in the forecast model). # - # NOTE: - # May want to remove kice from FV3.input.yml (and maybe input.nml.FV3). - # kice = None if SDF_USES_RUC_LSM: kice = 9 @@ -981,22 +965,6 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> logging.info( f""" - To launch the workflow, first ensure that you have a compatible version - of rocoto available. For most pre-configured platforms, rocoto can be - loaded via a module: - - > module load rocoto - - For more details on rocoto, see the User's Guide. - - To launch the workflow, first ensure that you have a compatible version - of rocoto loaded. For example, to load version 1.3.1 of rocoto, use - - > module load rocoto/1.3.1 - - (This version has been tested on hera; later versions may also work but - have not been tested.) - To launch the workflow, change location to the experiment directory (EXPTDIR) and issue the rocotrun command, as follows: diff --git a/ush/get_crontab_contents.py b/ush/get_crontab_contents.py index b0950f5c6f..da99596061 100644 --- a/ush/get_crontab_contents.py +++ b/ush/get_crontab_contents.py @@ -5,6 +5,7 @@ import unittest import argparse from datetime import datetime +from logging import getLogger from python_utils import ( import_vars, @@ -90,6 +91,7 @@ def get_crontab_contents(called_from_cron): def add_crontab_line(): """Add crontab line to cron table""" + logger = getLogger(__name__) # import selected env vars IMPORTS = ["MACHINE", "USER", "CRONTAB_LINE", "VERBOSE", "EXPTDIR"] import_vars(env_vars=IMPORTS) @@ -99,12 +101,11 @@ def add_crontab_line(): # time_stamp = datetime.now().strftime("%F_%T") crontab_backup_fp = os.path.join(EXPTDIR, f"crontab.bak.{time_stamp}") - print_info_msg( + logger.info(dedent( f''' Copying contents of user cron table to backup file: - crontab_backup_fp = \"{crontab_backup_fp}\"''', - verbose=VERBOSE, - ) + crontab_backup_fp = \"{crontab_backup_fp}\"''' + )) global called_from_cron try: @@ -123,22 +124,21 @@ def add_crontab_line(): # Add crontab line if CRONTAB_LINE in crontab_contents: - print_info_msg( + logger.info(dedent( f''' The following line already exists in the cron table and thus will not be added: CRONTAB_LINE = \"{CRONTAB_LINE}\"''' - ) + )) else: - print_info_msg( + logger.info(dedent( f''' Adding the following line to the user's cron table in order to automatically resubmit SRW workflow: - CRONTAB_LINE = \"{CRONTAB_LINE}\"''', - verbose=VERBOSE, - ) + CRONTAB_LINE = \"{CRONTAB_LINE}\"''' + )) # add new line to crontab contents if it doesn't have one NEWLINE_CHAR = "" diff --git a/ush/python_utils/check_for_preexist_dir_file.py b/ush/python_utils/check_for_preexist_dir_file.py index fc67447fb5..ca19c1b08c 100644 --- a/ush/python_utils/check_for_preexist_dir_file.py +++ b/ush/python_utils/check_for_preexist_dir_file.py @@ -2,7 +2,8 @@ import os from datetime import datetime -from .print_msg import print_info_msg, print_err_msg_exit +from logging import getLogger +from textwrap import dedent from .check_var_valid_value import check_var_valid_value from .filesys_cmds_vrfy import rm_vrfy, mv_vrfy @@ -27,17 +28,17 @@ def check_for_preexist_dir_file(path, method): now = datetime.now() d = now.strftime("_old_%Y%m%d_%H%M%S") new_path = path + d - print_info_msg( + logger.info(dedent( f""" Specified directory or file already exists: {path} Moving (renaming) preexisting directory or file to: {new_path}""" - ) + )) mv_vrfy(path, new_path) else: - print_err_msg_exit( + raise FileExistsError(dedent( f""" Specified directory or file already exists {path}""" - ) + )) diff --git a/ush/setup.py b/ush/setup.py index 02f16023b1..56777275ae 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -733,7 +733,18 @@ def get_location(xcs,fmt): # global EXPTDIR EXPTDIR = os.path.join(EXPT_BASEDIR, EXPT_SUBDIR) - check_for_preexist_dir_file(EXPTDIR, PREEXISTING_DIR_METHOD) + try: + check_for_preexist_dir_file(EXPTDIR, PREEXISTING_DIR_METHOD) + except FileExistsError: + errmsg = dedent(f''' + {EXPTDIR=} already exists, and {PREEXISTING_DIR_METHOD=} + + To ignore this error, delete the directory, or set + PREEXISTING_DIR_METHOD = delete, or + PREEXISTING_DIR_METHOD = rename + in your config file. + ''') + raise Exception(errmsg) from None # # ----------------------------------------------------------------------- # From fe53633bfd1321fdc67760c9f1d7f72a326c56d6 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Sat, 15 Oct 2022 05:16:49 +0000 Subject: [PATCH 13/26] First big change from more extensive testing: Bug in check_var_valid_value.py caused an unrelated exception if the intended check failed and err_msg was not specified. Turns out that nowhere in the code is err_msg used, so let's get rid of it! In addition, trying to check an empty string caused an *additional* exception, so added some logic to handle that case. --- ush/python_utils/check_for_preexist_dir_file.py | 9 ++++++++- ush/python_utils/check_var_valid_value.py | 16 +++++----------- ush/setup.py | 9 ++++++++- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/ush/python_utils/check_for_preexist_dir_file.py b/ush/python_utils/check_for_preexist_dir_file.py index ca19c1b08c..8bfb09cb9a 100644 --- a/ush/python_utils/check_for_preexist_dir_file.py +++ b/ush/python_utils/check_for_preexist_dir_file.py @@ -19,7 +19,14 @@ def check_for_preexist_dir_file(path, method): None """ - check_var_valid_value(method, ["delete", "rename", "quit"]) + try: + check_var_valid_value(method, ["delete", "rename", "quit"]) + except ValueError: + errmsg = dedent(f''' + Invalid method for dealing with pre-existing directory specified + {method=} + ''') + raise ValueError(errmsg) from None if os.path.exists(path): if method == "delete": diff --git a/ush/python_utils/check_var_valid_value.py b/ush/python_utils/check_var_valid_value.py index 10d8365cb6..e9b6d5e5fc 100644 --- a/ush/python_utils/check_var_valid_value.py +++ b/ush/python_utils/check_var_valid_value.py @@ -1,23 +1,17 @@ #!/usr/bin/env python3 -from .print_msg import print_err_msg_exit - - -def check_var_valid_value(var, values, err_msg=None): +def check_var_valid_value(var, values): """Check if specified variable has a valid value Args: var: the variable values: list of valid values - err_msg: additional error message to print Returns: True: if var has valid value, exit(1) otherwise """ - if var not in values: - if err_msg is not None: - err_msg = f"The value specified in var = {var} is not supported." - print_err_msg_exit( - err_msg + f"{var} must be set to one of the following:\n {values}" - ) + if not var: + var = "" + if not var or var not in values: + raise ValueError(f"Got '{var}', expected one of the following:\n {values}") return True diff --git a/ush/setup.py b/ush/setup.py index 56777275ae..e2a92cdfa7 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -735,6 +735,13 @@ def get_location(xcs,fmt): EXPTDIR = os.path.join(EXPT_BASEDIR, EXPT_SUBDIR) try: check_for_preexist_dir_file(EXPTDIR, PREEXISTING_DIR_METHOD) + except ValueError: + logger.exception(f''' + Check that the following values are valid: + {EXPTDIR=} + {PREEXISTING_DIR_METHOD=} + ''') + raise except FileExistsError: errmsg = dedent(f''' {EXPTDIR=} already exists, and {PREEXISTING_DIR_METHOD=} @@ -744,7 +751,7 @@ def get_location(xcs,fmt): PREEXISTING_DIR_METHOD = rename in your config file. ''') - raise Exception(errmsg) from None + raise FileExistsError(errmsg) from None # # ----------------------------------------------------------------------- # From a91df94602f236ec01bd9af7c76428e92de3c8f6 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Sat, 15 Oct 2022 05:25:59 +0000 Subject: [PATCH 14/26] Add testing subprocess code back in per Daniel Abdi's advice; this is necessary for CI testing --- ush/generate_FV3LAM_wflow.py | 9 +++++++++ ush/python_utils/check_var_valid_value.py | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 0139776bd7..63379251d5 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -1046,6 +1046,15 @@ def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: class Testing(unittest.TestCase): def test_generate_FV3LAM_wflow(self): + # run workflows in separate process to avoid conflict between community and nco settings + def run_workflow(): + p = Process(target=generate_FV3LAM_wflow) + p.start() + p.join() + exit_code = p.exitcode + if exit_code != 0: + sys.exit(exit_code) + USHdir = os.path.dirname(os.path.abspath(__file__)) logfile='log.generate_FV3LAM_wflow' SED = get_env_var("SED") diff --git a/ush/python_utils/check_var_valid_value.py b/ush/python_utils/check_var_valid_value.py index e9b6d5e5fc..a8b88e17a6 100644 --- a/ush/python_utils/check_var_valid_value.py +++ b/ush/python_utils/check_var_valid_value.py @@ -12,6 +12,6 @@ def check_var_valid_value(var, values): if not var: var = "" - if not var or var not in values: + if var not in values: raise ValueError(f"Got '{var}', expected one of the following:\n {values}") return True From 2ba941a33372aa15d2ce517a7d67bc97f1007513 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Sat, 15 Oct 2022 05:32:46 +0000 Subject: [PATCH 15/26] Forgot to set up logger for check_for_preexist_dir_file.py; not sure why this didn't fail in manual tests... --- ush/python_utils/check_for_preexist_dir_file.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ush/python_utils/check_for_preexist_dir_file.py b/ush/python_utils/check_for_preexist_dir_file.py index 8bfb09cb9a..a89b190f65 100644 --- a/ush/python_utils/check_for_preexist_dir_file.py +++ b/ush/python_utils/check_for_preexist_dir_file.py @@ -19,6 +19,8 @@ def check_for_preexist_dir_file(path, method): None """ + logger = getLogger(__name__) + try: check_var_valid_value(method, ["delete", "rename", "quit"]) except ValueError: From a90be2d3732b5ee05a1b8bd575aae3f6c9080438 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Sat, 15 Oct 2022 05:43:02 +0000 Subject: [PATCH 16/26] More fixes to Daniels tests --- ush/generate_FV3LAM_wflow.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 63379251d5..67c1063b93 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -1047,8 +1047,8 @@ class Testing(unittest.TestCase): def test_generate_FV3LAM_wflow(self): # run workflows in separate process to avoid conflict between community and nco settings - def run_workflow(): - p = Process(target=generate_FV3LAM_wflow) + def run_workflow(USHdir,logfile): + p = Process(target=generate_FV3LAM_wflow,args=(USHdir,logfile)) p.start() p.join() exit_code = p.exitcode @@ -1062,13 +1062,13 @@ def run_workflow(): # community test case cp_vrfy(f"{USHdir}/config.community.yaml", f"{USHdir}/config.yaml") run_command(f"""{SED} -i 's/MACHINE: hera/MACHINE: linux/g' {USHdir}/config.yaml""") - generate_FV3LAM_wflow(USHdir, logfile) + run_workflow(USHdir, logfile) # nco test case set_env_var("OPSROOT", f"{USHdir}/../../nco_dirs") cp_vrfy(f"{USHdir}/config.nco.yaml", f"{USHdir}/config.yaml") run_command(f"""{SED} -i 's/MACHINE: hera/MACHINE: linux/g' {USHdir}/config.yaml""") - generate_FV3LAM_wflow(USHdir, logfile) + run_workflow(USHdir, logfile) def setUp(self): define_macos_utilities() From 6389c76ce54df8b34fea3fb4addc1d3ea24f1f24 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Wed, 19 Oct 2022 16:49:39 +0000 Subject: [PATCH 17/26] Add missing import to fix failing test --- ush/get_crontab_contents.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ush/get_crontab_contents.py b/ush/get_crontab_contents.py index da99596061..970fe4ba15 100644 --- a/ush/get_crontab_contents.py +++ b/ush/get_crontab_contents.py @@ -6,6 +6,7 @@ import argparse from datetime import datetime from logging import getLogger +from textwrap import dedent from python_utils import ( import_vars, From c76df4fc8adb6d3e3a15031f074545d39fdca73a Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Wed, 19 Oct 2022 21:12:08 +0000 Subject: [PATCH 18/26] Address some comments from Daniel: implement logging.info as its own function with built-in dedenting. --- ush/generate_FV3LAM_wflow.py | 55 ++++++++----------- ush/get_crontab_contents.py | 15 +++-- ush/python_utils/__init__.py | 2 +- .../check_for_preexist_dir_file.py | 9 +-- ush/python_utils/print_msg.py | 21 ++++++- ush/set_ozone_param.py | 17 ++---- ush/set_thompson_mp_fix_files.py | 13 ++--- ush/setup.py | 15 ++--- 8 files changed, 72 insertions(+), 75 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 67c1063b93..959cdef26a 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -13,6 +13,7 @@ from python_utils import ( print_info_msg, print_err_msg_exit, + log_info, import_vars, cp_vrfy, cd_vrfy, @@ -76,13 +77,11 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # Set up logging to write to screen and logfile setup_logging(logfile) - logging.info( - dedent( + log_info( """ ======================================================================== Starting experiment generation... ========================================================================""" - ) ) # check python version @@ -132,7 +131,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> template_xml_fp = os.path.join(PARMdir, WFLOW_XML_FN) - logging.info( + log_info( f''' Creating rocoto workflow XML file (WFLOW_XML_FP) from jinja template XML file (template_xml_fp): @@ -427,16 +426,13 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # End of "settings" variable. settings_str = cfg_to_yaml_str(settings) - logging.info( - dedent( + log_info( f""" The variable \"settings\" specifying values of the rococo XML variables has been set as follows: #----------------------------------------------------------------------- - settings =\n\n""" - ) - + settings_str, - ) + settings =\n\n""") + log_info(settings_str) # # Call the python script to generate the experiment's actual XML file @@ -470,7 +466,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # # ----------------------------------------------------------------------- # - logging.info( + log_info( f''' Creating symlink in the experiment directory (EXPTDIR) that points to the workflow launch script (WFLOW_LAUNCH_SCRIPT_FP): @@ -498,7 +494,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # if SYMLINK_FIX_FILES: - logging.info( + log_info( f''' Symlinking fixed files from system directory (FIXgsm) to a subdirectory (FIXam): FIXgsm = \"{FIXgsm}\" @@ -508,7 +504,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> ln_vrfy(f'''-fsn "{FIXgsm}" "{FIXam}"''') else: - logging.info( + log_info( f''' Copying fixed files from system directory (FIXgsm) to a subdirectory (FIXam): FIXgsm = \"{FIXgsm}\" @@ -531,7 +527,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # ----------------------------------------------------------------------- # if USE_MERRA_CLIMO: - logging.info( + log_info( f''' Copying MERRA2 aerosol climatology data files from system directory (FIXaer/FIXlut) to a subdirectory (FIXclim) in the experiment directory: @@ -556,24 +552,24 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # # ----------------------------------------------------------------------- # - logging.info( + log_info( f""" Copying templates of various input files to the experiment directory...""", ) - logging.info( + log_info( f""" Copying the template data table file to the experiment directory...""", ) cp_vrfy(DATA_TABLE_TMPL_FP, DATA_TABLE_FP) - logging.info( + log_info( f""" Copying the template field table file to the experiment directory...""", ) cp_vrfy(FIELD_TABLE_TMPL_FP, FIELD_TABLE_FP) - logging.info( + log_info( f""" Copying the template NEMS configuration file to the experiment directory...""", ) @@ -583,7 +579,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # clone of the FV3 code repository to the experiment directory (EXPT- # DIR). # - logging.info( + log_info( f""" Copying the CCPP physics suite definition XML file from its location in the forecast model directory sturcture to the experiment directory...""", @@ -594,7 +590,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # clone of the FV3 code repository to the experiment directory (EXPT- # DIR). # - logging.info( + log_info( f""" Copying the field dictionary file from its location in the forecast model directory sturcture to the experiment directory...""", @@ -607,7 +603,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # # ----------------------------------------------------------------------- # - logging.info( + log_info( f''' Setting parameters in weather model's namelist file (FV3_NML_FP): FV3_NML_FP = \"{FV3_NML_FP}\"''' @@ -839,16 +835,11 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> settings_str = cfg_to_yaml_str(settings) - logging.info( - dedent( + log_info( f""" The variable \"settings\" specifying values of the weather model's - namelist variables has been set as follows: - - settings =\n\n""" - ) - + settings_str, - ) + namelist variables has been set as follows:\n""") + log_info("\nsettings =\n\n" + settings_str) # # ----------------------------------------------------------------------- # @@ -941,7 +932,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> rocotorun_cmd = f"rocotorun -w {WFLOW_XML_FN} -d {wflow_db_fn} -v 10" rocotostat_cmd = f"rocotostat -w {WFLOW_XML_FN} -d {wflow_db_fn} -v 10" - logging.info( + log_info( f""" ======================================================================== ======================================================================== @@ -963,7 +954,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> # if WORKFLOW_MANAGER == "rocoto": - logging.info( + log_info( f""" To launch the workflow, change location to the experiment directory (EXPTDIR) and issue the rocotrun command, as follows: @@ -1012,7 +1003,7 @@ def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: messages with detailed timing and routine info in the specified text file. """ logging.basicConfig(level=logging.DEBUG, - format='%(name)-12s %(levelname)-8s %(message)s', + format='%(name)-22s %(levelname)-8s %(message)s', filename=logfile, filemode='w') logging.debug(f'Finished setting up debug file logging in {logfile}') diff --git a/ush/get_crontab_contents.py b/ush/get_crontab_contents.py index 970fe4ba15..fa21ae3503 100644 --- a/ush/get_crontab_contents.py +++ b/ush/get_crontab_contents.py @@ -5,10 +5,10 @@ import unittest import argparse from datetime import datetime -from logging import getLogger from textwrap import dedent from python_utils import ( + log_info, import_vars, set_env_var, print_input_args, @@ -92,7 +92,6 @@ def get_crontab_contents(called_from_cron): def add_crontab_line(): """Add crontab line to cron table""" - logger = getLogger(__name__) # import selected env vars IMPORTS = ["MACHINE", "USER", "CRONTAB_LINE", "VERBOSE", "EXPTDIR"] import_vars(env_vars=IMPORTS) @@ -102,11 +101,11 @@ def add_crontab_line(): # time_stamp = datetime.now().strftime("%F_%T") crontab_backup_fp = os.path.join(EXPTDIR, f"crontab.bak.{time_stamp}") - logger.info(dedent( + log_info( f''' Copying contents of user cron table to backup file: crontab_backup_fp = \"{crontab_backup_fp}\"''' - )) + ) global called_from_cron try: @@ -125,21 +124,21 @@ def add_crontab_line(): # Add crontab line if CRONTAB_LINE in crontab_contents: - logger.info(dedent( + log_info( f''' The following line already exists in the cron table and thus will not be added: CRONTAB_LINE = \"{CRONTAB_LINE}\"''' - )) + ) else: - logger.info(dedent( + log_info( f''' Adding the following line to the user's cron table in order to automatically resubmit SRW workflow: CRONTAB_LINE = \"{CRONTAB_LINE}\"''' - )) + ) # add new line to crontab contents if it doesn't have one NEWLINE_CHAR = "" diff --git a/ush/python_utils/__init__.py b/ush/python_utils/__init__.py index 8a74a595d0..add96ee981 100644 --- a/ush/python_utils/__init__.py +++ b/ush/python_utils/__init__.py @@ -28,7 +28,7 @@ from .get_elem_inds import get_elem_inds from .interpol_to_arbit_CRES import interpol_to_arbit_CRES from .print_input_args import print_input_args -from .print_msg import print_info_msg, print_err_msg_exit +from .print_msg import print_info_msg, print_err_msg_exit, log_info from .run_command import run_command from .get_charvar_from_netcdf import get_charvar_from_netcdf from .xml_parser import load_xml_file, has_tag_with_value diff --git a/ush/python_utils/check_for_preexist_dir_file.py b/ush/python_utils/check_for_preexist_dir_file.py index a89b190f65..a6d006c9a1 100644 --- a/ush/python_utils/check_for_preexist_dir_file.py +++ b/ush/python_utils/check_for_preexist_dir_file.py @@ -2,11 +2,10 @@ import os from datetime import datetime -from logging import getLogger from textwrap import dedent from .check_var_valid_value import check_var_valid_value from .filesys_cmds_vrfy import rm_vrfy, mv_vrfy - +from .print_msg import log_info def check_for_preexist_dir_file(path, method): """Check for a preexisting directory or file and, if present, deal with it @@ -19,8 +18,6 @@ def check_for_preexist_dir_file(path, method): None """ - logger = getLogger(__name__) - try: check_var_valid_value(method, ["delete", "rename", "quit"]) except ValueError: @@ -37,13 +34,13 @@ def check_for_preexist_dir_file(path, method): now = datetime.now() d = now.strftime("_old_%Y%m%d_%H%M%S") new_path = path + d - logger.info(dedent( + log_info( f""" Specified directory or file already exists: {path} Moving (renaming) preexisting directory or file to: {new_path}""" - )) + ) mv_vrfy(path, new_path) else: raise FileExistsError(dedent( diff --git a/ush/python_utils/print_msg.py b/ush/python_utils/print_msg.py index 606284e140..ce84452042 100644 --- a/ush/python_utils/print_msg.py +++ b/ush/python_utils/print_msg.py @@ -2,7 +2,8 @@ import traceback import sys -from textwrap import dedent +from textwrap import dedent, indent +from logging import getLogger def print_err_msg_exit(error_msg="", stack_trace=True): @@ -39,3 +40,21 @@ def print_info_msg(info_msg, verbose=True): print(dedent(info_msg)) return True return False + +def log_info(info_msg, ddent=True): + """Function to print information message using the logging module. This function + should not be used if python logging has not been initialized. + + Args: + info_msg : info message to print + ddent : set to False to disable "dedenting" (print string as-is) + Returns: + None + """ + + logger=getLogger(sys._getframe().f_back.f_code.co_name) + if ddent: + logger.info(indent(dedent(info_msg), ' ')) + else: + logger.info(info_msg) + diff --git a/ush/set_ozone_param.py b/ush/set_ozone_param.py index 7a7a4ae75c..e8d4c7a7cb 100644 --- a/ush/set_ozone_param.py +++ b/ush/set_ozone_param.py @@ -3,9 +3,9 @@ import os import unittest from textwrap import dedent -from logging import getLogger from python_utils import ( + log_info, import_vars, export_vars, set_env_var, @@ -45,7 +45,6 @@ def set_ozone_param(ccpp_phys_suite_fp): ozone_param: a string """ - logger = getLogger(__name__) print_input_args(locals()) # import all environment variables @@ -149,23 +148,17 @@ def set_ozone_param(ccpp_phys_suite_fp): # ----------------------------------------------------------------------- # if fixgsm_ozone_fn_is_set: - - msg = dedent( + log_info( f""" After setting the file name of the ozone production/loss file in the FIXgsm directory (based on the ozone parameterization specified in the CCPP suite definition file), the array specifying the mapping between the symlinks that need to be created in the cycle directories and the files in the FIXam directory is: - - """ - ) - msg += dedent( - f""" + """) + log_info(f""" CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING = {list_to_str(CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING)} - """ - ) - logger.info(msg) + """, ddent=False) else: diff --git a/ush/set_thompson_mp_fix_files.py b/ush/set_thompson_mp_fix_files.py index 76f230d628..93dc3c5de6 100644 --- a/ush/set_thompson_mp_fix_files.py +++ b/ush/set_thompson_mp_fix_files.py @@ -3,9 +3,9 @@ import os import unittest from textwrap import dedent -from logging import getLogger from python_utils import ( + log_info, import_vars, export_vars, set_env_var, @@ -35,7 +35,6 @@ def set_thompson_mp_fix_files(ccpp_phys_suite_fp, thompson_mp_climo_fn): boolean: sdf_uses_thompson_mp """ - logger = getLogger(__name__) print_input_args(locals()) # import all environment variables @@ -90,7 +89,7 @@ def set_thompson_mp_fix_files(ccpp_phys_suite_fp, thompson_mp_climo_fn): mapping = f"{fix_file} | {fix_file}" CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING.append(mapping) - msg = dedent( + log_info( f""" Since the Thompson microphysics parameterization is being used by this physics suite (CCPP_PHYS_SUITE), the names of the fixed files needed by @@ -100,21 +99,19 @@ def set_thompson_mp_fix_files(ccpp_phys_suite_fp, thompson_mp_climo_fn): CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING. After these modifications, the values of these parameters are as follows: + CCPP_PHYS_SUITE = \"{CCPP_PHYS_SUITE}\" """ ) - msg += dedent( + log_info( f""" - CCPP_PHYS_SUITE = \"{CCPP_PHYS_SUITE}\" - FIXgsm_FILES_TO_COPY_TO_FIXam = {list_to_str(FIXgsm_FILES_TO_COPY_TO_FIXam)} """ ) - msg += dedent( + log_info( f""" CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING = {list_to_str(CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING)} """ ) - logger.info(msg) EXPORTS = [ "CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING", diff --git a/ush/setup.py b/ush/setup.py index e2a92cdfa7..15b542a3c4 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -7,6 +7,7 @@ from logging import getLogger from python_utils import ( + log_info, cd_vrfy, mkdir_vrfy, rm_vrfy, @@ -64,12 +65,12 @@ def setup(): import_vars() # print message - logger.info(dedent( + log_info( f""" ======================================================================== Starting function setup() in \"{os.path.basename(__file__)}\"... ========================================================================""" - )) + ) # # ----------------------------------------------------------------------- # @@ -193,7 +194,7 @@ def get_location(xcs,fmt): # global WORKFLOW_ID WORKFLOW_ID = "id_" + str(int(datetime.datetime.now().timestamp())) - logger.info(f"""WORKFLOW ID = {WORKFLOW_ID}""") + log_info(f"""WORKFLOW ID = {WORKFLOW_ID}""") # # ----------------------------------------------------------------------- @@ -220,7 +221,7 @@ def get_location(xcs,fmt): # global VERBOSE if DEBUG and not VERBOSE: - logger.info( + log_info( """ Resetting VERBOSE to \"TRUE\" because DEBUG has been set to \"TRUE\"...""" ) @@ -1503,7 +1504,7 @@ def get_location(xcs,fmt): if QUILTING: PE_MEMBER01 = PE_MEMBER01 + WRTCMP_write_groups * WRTCMP_write_tasks_per_group - logger.info( + log_info( f""" The number of MPI tasks for the forecast (including those for the write component if it is being used) are: @@ -1844,10 +1845,10 @@ def get_location(xcs,fmt): # print content of var_defns if DEBUG=True if DEBUG: all_lines = cfg_to_yaml_str(cfg_d) - logger.info(all_lines) + log_info(all_lines) # print info message - logger.info( + log_info( f""" Generating the global experiment variable definitions file specified by GLOBAL_VAR_DEFNS_FN: From 3835b9b7bf00984b394cb53dc9762bdc7713ff4d Mon Sep 17 00:00:00 2001 From: "Michael J. Kavulich, Jr" Date: Wed, 19 Oct 2022 22:18:22 +0000 Subject: [PATCH 19/26] Fail with error if model executable does not exist --- ush/setup.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ush/setup.py b/ush/setup.py index 15b542a3c4..83741a1977 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -1123,6 +1123,17 @@ def get_location(xcs,fmt): # Set the full path to the forecast model executable. global FV3_EXEC_FP FV3_EXEC_FP = os.path.join(EXECdir, FV3_EXEC_FN) + if not os.path.exists(FV3_EXEC_FP): + raise FileNotFoundError(dedent( + f''' + The forecast model executable + + {FV3_EXEC_FP=} + + does not exist! Ensure you have successfully built the SRW App executables + prior to running this script!''' + )) + # # ----------------------------------------------------------------------- # From 6f56ed648a032a88b2e22f1e3addbd982097d472 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Wed, 19 Oct 2022 22:28:16 +0000 Subject: [PATCH 20/26] Revert "Fail with error if model executable does not exist" This reverts commit 252f91dda4d84bf4417d2af324282fd730625732. --- ush/python_utils/print_msg.py | 1 + ush/setup.py | 11 ----------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/ush/python_utils/print_msg.py b/ush/python_utils/print_msg.py index ce84452042..d437e67de9 100644 --- a/ush/python_utils/print_msg.py +++ b/ush/python_utils/print_msg.py @@ -52,6 +52,7 @@ def log_info(info_msg, ddent=True): None """ + # "sys._getframe().f_back.f_code.co_name" returns the name of the calling function logger=getLogger(sys._getframe().f_back.f_code.co_name) if ddent: logger.info(indent(dedent(info_msg), ' ')) diff --git a/ush/setup.py b/ush/setup.py index 83741a1977..15b542a3c4 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -1123,17 +1123,6 @@ def get_location(xcs,fmt): # Set the full path to the forecast model executable. global FV3_EXEC_FP FV3_EXEC_FP = os.path.join(EXECdir, FV3_EXEC_FN) - if not os.path.exists(FV3_EXEC_FP): - raise FileNotFoundError(dedent( - f''' - The forecast model executable - - {FV3_EXEC_FP=} - - does not exist! Ensure you have successfully built the SRW App executables - prior to running this script!''' - )) - # # ----------------------------------------------------------------------- # From 5b7093e09324c46e8c7c3a28deeae6a8f12c0052 Mon Sep 17 00:00:00 2001 From: "Michael J. Kavulich, Jr" Date: Thu, 20 Oct 2022 00:36:34 +0000 Subject: [PATCH 21/26] Restore "verbose" functionality now that we have a function --- ush/generate_FV3LAM_wflow.py | 20 +++++++++++++++++--- ush/get_crontab_contents.py | 6 ++++-- ush/python_utils/print_msg.py | 13 ++++++++----- ush/set_ozone_param.py | 4 ++-- ush/setup.py | 9 +++++---- 5 files changed, 36 insertions(+), 16 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 959cdef26a..d8ea60c13e 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -431,8 +431,10 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> The variable \"settings\" specifying values of the rococo XML variables has been set as follows: #----------------------------------------------------------------------- - settings =\n\n""") - log_info(settings_str) + settings =\n\n""", + verbose=VERBOSE, + ) + log_info(settings_str, verbose=VERBOSE) # # Call the python script to generate the experiment's actual XML file @@ -472,6 +474,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> workflow launch script (WFLOW_LAUNCH_SCRIPT_FP): EXPTDIR = \"{EXPTDIR}\" WFLOW_LAUNCH_SCRIPT_FP = \"{WFLOW_LAUNCH_SCRIPT_FP}\"''', + verbose=VERBOSE, ) create_symlink_to_file( @@ -499,6 +502,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> Symlinking fixed files from system directory (FIXgsm) to a subdirectory (FIXam): FIXgsm = \"{FIXgsm}\" FIXam = \"{FIXam}\"''', + verbose=VERBOSE, ) ln_vrfy(f'''-fsn "{FIXgsm}" "{FIXam}"''') @@ -509,6 +513,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> Copying fixed files from system directory (FIXgsm) to a subdirectory (FIXam): FIXgsm = \"{FIXgsm}\" FIXam = \"{FIXam}\"''', + verbose=VERBOSE, ) check_for_preexist_dir_file(FIXam, "delete") @@ -534,6 +539,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> FIXaer = \"{FIXaer}\" FIXlut = \"{FIXlut}\" FIXclim = \"{FIXclim}\"''', + verbose=VERBOSE, ) check_for_preexist_dir_file(FIXclim, "delete") @@ -555,23 +561,27 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> log_info( f""" Copying templates of various input files to the experiment directory...""", + verbose=VERBOSE, ) log_info( f""" Copying the template data table file to the experiment directory...""", + verbose=VERBOSE, ) cp_vrfy(DATA_TABLE_TMPL_FP, DATA_TABLE_FP) log_info( f""" Copying the template field table file to the experiment directory...""", + verbose=VERBOSE, ) cp_vrfy(FIELD_TABLE_TMPL_FP, FIELD_TABLE_FP) log_info( f""" Copying the template NEMS configuration file to the experiment directory...""", + verbose=VERBOSE, ) cp_vrfy(NEMS_CONFIG_TMPL_FP, NEMS_CONFIG_FP) # @@ -583,6 +593,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> f""" Copying the CCPP physics suite definition XML file from its location in the forecast model directory sturcture to the experiment directory...""", + verbose=VERBOSE, ) cp_vrfy(CCPP_PHYS_SUITE_IN_CCPP_FP, CCPP_PHYS_SUITE_FP) # @@ -594,6 +605,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> f""" Copying the field dictionary file from its location in the forecast model directory sturcture to the experiment directory...""", + verbose=VERBOSE, ) cp_vrfy(FIELD_DICT_IN_UWM_FP, FIELD_DICT_FP) # @@ -838,7 +850,9 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> log_info( f""" The variable \"settings\" specifying values of the weather model's - namelist variables has been set as follows:\n""") + namelist variables has been set as follows:\n""", + verbose=VERBOSE, + ) log_info("\nsettings =\n\n" + settings_str) # # ----------------------------------------------------------------------- diff --git a/ush/get_crontab_contents.py b/ush/get_crontab_contents.py index fa21ae3503..c6c8d41ee0 100644 --- a/ush/get_crontab_contents.py +++ b/ush/get_crontab_contents.py @@ -104,7 +104,8 @@ def add_crontab_line(): log_info( f''' Copying contents of user cron table to backup file: - crontab_backup_fp = \"{crontab_backup_fp}\"''' + crontab_backup_fp = \"{crontab_backup_fp}\"''', + verbose=VERBOSE, ) global called_from_cron @@ -137,7 +138,8 @@ def add_crontab_line(): f''' Adding the following line to the user's cron table in order to automatically resubmit SRW workflow: - CRONTAB_LINE = \"{CRONTAB_LINE}\"''' + CRONTAB_LINE = \"{CRONTAB_LINE}\"''', + verbose=VERBOSE, ) # add new line to crontab contents if it doesn't have one diff --git a/ush/python_utils/print_msg.py b/ush/python_utils/print_msg.py index d437e67de9..9f38998c33 100644 --- a/ush/python_utils/print_msg.py +++ b/ush/python_utils/print_msg.py @@ -41,12 +41,13 @@ def print_info_msg(info_msg, verbose=True): return True return False -def log_info(info_msg, ddent=True): +def log_info(info_msg, verbose=True, ddent=True): """Function to print information message using the logging module. This function should not be used if python logging has not been initialized. Args: info_msg : info message to print + verbose : set to False to silence printing ddent : set to False to disable "dedenting" (print string as-is) Returns: None @@ -54,8 +55,10 @@ def log_info(info_msg, ddent=True): # "sys._getframe().f_back.f_code.co_name" returns the name of the calling function logger=getLogger(sys._getframe().f_back.f_code.co_name) - if ddent: - logger.info(indent(dedent(info_msg), ' ')) - else: - logger.info(info_msg) + + if verbose: + if ddent: + logger.info(indent(dedent(info_msg), ' ')) + else: + logger.info(info_msg) diff --git a/ush/set_ozone_param.py b/ush/set_ozone_param.py index e8d4c7a7cb..0bbbf06d47 100644 --- a/ush/set_ozone_param.py +++ b/ush/set_ozone_param.py @@ -155,10 +155,10 @@ def set_ozone_param(ccpp_phys_suite_fp): CCPP suite definition file), the array specifying the mapping between the symlinks that need to be created in the cycle directories and the files in the FIXam directory is: - """) + """, verbose=VERBOSE) log_info(f""" CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING = {list_to_str(CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING)} - """, ddent=False) + """, verbose=VERBOSE, ddent=False) else: diff --git a/ush/setup.py b/ush/setup.py index 15b542a3c4..2a3081daeb 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -1504,11 +1504,13 @@ def get_location(xcs,fmt): if QUILTING: PE_MEMBER01 = PE_MEMBER01 + WRTCMP_write_groups * WRTCMP_write_tasks_per_group - log_info( + if VERBOSE: + log_info( f""" The number of MPI tasks for the forecast (including those for the write component if it is being used) are: PE_MEMBER01 = {PE_MEMBER01}""", + verbose=VERBOSE, ) # # ----------------------------------------------------------------------- @@ -1843,9 +1845,8 @@ def get_location(xcs,fmt): # # print content of var_defns if DEBUG=True - if DEBUG: - all_lines = cfg_to_yaml_str(cfg_d) - log_info(all_lines) + all_lines = cfg_to_yaml_str(cfg_d) + log_info(all_lines, verbose=DEBUG) # print info message log_info( From 2d4f5b016d462061aa996fff9a46bb91a4b437ec Mon Sep 17 00:00:00 2001 From: "Michael J. Kavulich, Jr" Date: Thu, 20 Oct 2022 00:59:36 +0000 Subject: [PATCH 22/26] Missed one VERBOSE pass --- ush/generate_FV3LAM_wflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index d8ea60c13e..4ee147f827 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -853,7 +853,7 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> namelist variables has been set as follows:\n""", verbose=VERBOSE, ) - log_info("\nsettings =\n\n" + settings_str) + log_info("\nsettings =\n\n" + settings_str, verbose=VERBOSE) # # ----------------------------------------------------------------------- # From d2c9cbfb1d82c6ea159905e0e1e21bdc6715224d Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Thu, 20 Oct 2022 02:08:30 +0000 Subject: [PATCH 23/26] The f-string "=" specifier is not supported for python 3.6 (our minimum requirement); reverting to older format --- .../check_for_preexist_dir_file.py | 2 +- ush/set_predef_grid_params.py | 2 +- ush/setup.py | 48 +++++++++---------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/ush/python_utils/check_for_preexist_dir_file.py b/ush/python_utils/check_for_preexist_dir_file.py index a6d006c9a1..cfd4d50f43 100644 --- a/ush/python_utils/check_for_preexist_dir_file.py +++ b/ush/python_utils/check_for_preexist_dir_file.py @@ -23,7 +23,7 @@ def check_for_preexist_dir_file(path, method): except ValueError: errmsg = dedent(f''' Invalid method for dealing with pre-existing directory specified - {method=} + method = {method} ''') raise ValueError(errmsg) from None diff --git a/ush/set_predef_grid_params.py b/ush/set_predef_grid_params.py index 226d17065c..6b432b8f03 100644 --- a/ush/set_predef_grid_params.py +++ b/ush/set_predef_grid_params.py @@ -42,7 +42,7 @@ def set_predef_grid_params(): params_dict = params_dict[PREDEF_GRID_NAME] except KeyError: errmsg = dedent(f''' - {PREDEF_GRID_NAME=} not found in predef_grid_params.yaml + PREDEF_GRID_NAME = {PREDEF_GRID_NAME} not found in predef_grid_params.yaml Check your config file settings.''') raise Exception(errmsg) from None diff --git a/ush/setup.py b/ush/setup.py index 2a3081daeb..033180c760 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -92,7 +92,7 @@ def setup(): # variables that override their default values. Ensure all user-specified # variables correspond to a default value. if not os.path.exists(EXPT_CONFIG_FN): - raise FileNotFoundError(f'User config file not found: {EXPT_CONFIG_FN=}') + raise FileNotFoundError(f'User config file not found: EXPT_CONFIG_FN = {EXPT_CONFIG_FN}') try: cfg_u = load_config_file(os.path.join(USHdir, EXPT_CONFIG_FN)) @@ -134,7 +134,7 @@ def setup(): raise FileNotFoundError(dedent( f""" The machine file {MACHINE_FILE} does not exist. - Check that you have specified the correct machine ({MACHINE=}) in your config file {EXPT_CONFIG_FN}""" + Check that you have specified the correct machine ({MACHINE}) in your config file {EXPT_CONFIG_FN}""" )) machine_cfg = load_config_file(MACHINE_FILE) @@ -307,14 +307,14 @@ def get_location(xcs,fmt): All MYNN PBL, MYNN SFC, GSL GWD, Thompson MP, or RRTMG SPP-related namelist variables set in {EXPT_CONFIG_FN} must be equal in number of entries to what is found in SPP_VAR_LIST: - {len(SPP_VAR_LIST)=} - {len(SPP_MAG_LIST)=} - {len(SPP_LSCALE)=} - {len(SPP_TSCALE)=} - {len(SPP_SIGTOP1)=} - {len(SPP_SIGTOP2)=} - {len(SPP_STDDEV_CUTOFF)=} - {len(ISEED_SPP)=} + SPP_VAR_LIST (length {len(SPP_VAR_LIST)}) + SPP_MAG_LIST (length {len(SPP_MAG_LIST)}) + SPP_LSCALE (length {len(SPP_LSCALE)}) + SPP_TSCALE (length {len(SPP_TSCALE)}) + SPP_SIGTOP1 (length {len(SPP_SIGTOP1)}) + SPP_SIGTOP2 (length {len(SPP_SIGTOP2)}) + SPP_STDDEV_CUTOFF (length {len(SPP_STDDEV_CUTOFF)}) + ISEED_SPP (length {len(ISEED_SPP)}) ''' ) # @@ -337,10 +337,10 @@ def get_location(xcs,fmt): All Noah or RUC-LSM SPP-related namelist variables (except ISEED_LSM_SPP) set in {EXPT_CONFIG_FN} must be equal in number of entries to what is found in SPP_VAR_LIST: - {len(LSM_SPP_VAR_LIST)=} - {len(LSM_SPP_MAG_LIST)=} - {len(LSM_SPP_LSCALE)=} - {len(LSM_SPP_TSCALE)=} + LSM_SPP_VAR_LIST (length {len(LSM_SPP_VAR_LIST)}) + LSM_SPP_MAG_LIST (length {len(LSM_SPP_MAG_LIST)}) + LSM_SPP_LSCALE (length {len(LSM_SPP_LSCALE)}) + LSM_SPP_TSCALE (length {len(LSM_SPP_TSCALE)}) ''' ) # @@ -479,7 +479,7 @@ def get_location(xcs,fmt): if not ACCOUNT: raise Exception(dedent(f''' ACCOUNT must be specified in config or machine file if using a workflow manager. - {WORKFLOW_MANAGER=}\n''' + WORKFLOW_MANAGER = {WORKFLOW_MANAGER}\n''' )) # # ----------------------------------------------------------------------- @@ -522,7 +522,7 @@ def get_location(xcs,fmt): # Make sure RESTART_INTERVAL is set to an integer value if not isinstance(RESTART_INTERVAL, int): - raise Exception(f"\n{RESTART_INTERVAL=}, must be an integer value\n") + raise Exception(f"\nRESTART_INTERVAL = {RESTART_INTERVAL}, must be an integer value\n") # Check that input dates are in a date format @@ -547,7 +547,7 @@ def get_location(xcs,fmt): raise FileNotFoundError(dedent( f''' USE_CUSTOM_POST_CONFIG_FILE has been set, but the custom post configuration file - {CUSTOM_POST_CONFIG_FP=} + CUSTOM_POST_CONFIG_FP = {CUSTOM_POST_CONFIG_FP} could not be found.''' )) from None @@ -561,7 +561,7 @@ def get_location(xcs,fmt): raise FileNotFoundError(dedent( f''' USE_CRTM has been set, but the external CRTM fix file directory: - {CRTM_DIR=} + CRTM_DIR = {CRTM_DIR} could not be found.''' )) from None @@ -739,13 +739,13 @@ def get_location(xcs,fmt): except ValueError: logger.exception(f''' Check that the following values are valid: - {EXPTDIR=} - {PREEXISTING_DIR_METHOD=} + EXPTDIR {EXPTDIR} + PREEXISTING_DIR_METHOD {PREEXISTING_DIR_METHOD} ''') raise except FileExistsError: errmsg = dedent(f''' - {EXPTDIR=} already exists, and {PREEXISTING_DIR_METHOD=} + EXPTDIR ({EXPTDIR}) already exists, and PREEXISTING_DIR_METHOD = {PREEXISTING_DIR_METHOD} To ignore this error, delete the directory, or set PREEXISTING_DIR_METHOD = delete, or @@ -1221,7 +1221,7 @@ def get_location(xcs,fmt): msg = dedent(f""" GRID_DIR not specified! - Setting {GRID_DIR=} + Setting GRID_DIR = {GRID_DIR} """) logger.warning(msg) @@ -1246,7 +1246,7 @@ def get_location(xcs,fmt): msg = dedent(f""" OROG_DIR not specified! - Setting {OROG_DIR=} + Setting OROG_DIR = {OROG_DIR} """) logger.warning(msg) @@ -1271,7 +1271,7 @@ def get_location(xcs,fmt): msg = dedent(f""" SFC_CLIMO_DIR not specified! - Setting {SFC_CLIMO_DIR=} + Setting SFC_CLIMO_DIR ={SFC_CLIMO_DIR} """) logger.warning(msg) From 73a5c50db558bf5e7712124dc7d8d98b7e81cbcd Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Fri, 21 Oct 2022 21:03:59 +0000 Subject: [PATCH 24/26] Address reviewer comments --- ush/generate_FV3LAM_wflow.py | 17 ++++++++--------- ush/python_utils/config_parser.py | 7 ++----- ush/python_utils/print_msg.py | 6 +++--- ush/set_ozone_param.py | 10 +++------- ush/setup.py | 13 +++++++------ 5 files changed, 23 insertions(+), 30 deletions(-) diff --git a/ush/generate_FV3LAM_wflow.py b/ush/generate_FV3LAM_wflow.py index 4ee147f827..11a86c9aaa 100755 --- a/ush/generate_FV3LAM_wflow.py +++ b/ush/generate_FV3LAM_wflow.py @@ -917,9 +917,11 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> set_FV3nml_sfc_climo_filenames() - # Call script to get NOMADS data + # Call function to get NOMADS data if NOMADS: - get_nomads_data(NOMADS_file_type,EXPTDIR,USHdir,DATE_FIRST_CYCL,CYCL_HRS,FCST_LEN_HRS,LBC_SPEC_INTVL_HRS) + raise Exception("Nomads script does not work!") + + # get_nomads_data(NOMADS_file_type,EXPTDIR,USHdir,DATE_FIRST_CYCL,CYCL_HRS,FCST_LEN_HRS,LBC_SPEC_INTVL_HRS) # # ----------------------------------------------------------------------- @@ -959,10 +961,9 @@ def generate_FV3LAM_wflow(USHdir, logfile: str = 'log.generate_FV3LAM_wflow') -> ======================================================================== """ ) - # # ----------------------------------------------------------------------- # - # If rocoto is required, print instructions on how to load and use it + # If rocoto is required, print instructions on how to use it # # ----------------------------------------------------------------------- # @@ -1007,9 +1008,8 @@ def get_nomads_data(NOMADS_file_type,EXPTDIR,USHdir,DATE_FIRST_CYCL,CYCL_HRS,FCS print(f"NOMADS_file_type= {NOMADS_file_type}") cd_vrfy(EXPTDIR) NOMADS_script = os.path.join(USHdir, "NOMADS_get_extrn_mdl_files.sh") - # run_command(f"""{NOMADS_script} {date_to_str(DATE_FIRST_CYCL,format="%Y%m%d")} \ - # {date_to_str(DATE_FIRST_CYCL,format="%H")} {NOMADS_file_type} {FCST_LEN_HRS} {LBC_SPEC_INTVL_HRS}""") - raise Exception("Nomads script does not work") + run_command(f"""{NOMADS_script} {date_to_str(DATE_FIRST_CYCL,format="%Y%m%d")} \ + {date_to_str(DATE_FIRST_CYCL,format="%H")} {NOMADS_file_type} {FCST_LEN_HRS} {LBC_SPEC_INTVL_HRS}""") def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: """ @@ -1036,11 +1036,10 @@ def setup_logging(logfile: str = 'log.generate_FV3LAM_wflow') -> None: try: generate_FV3LAM_wflow(USHdir, logfile) except: - # If the call to the generate_FV3LAM_wflow function above was not successful, - # print out an error message and exit with a nonzero return code. logging.exception(dedent( f""" ********************************************************************* + FATAL ERROR: Experiment generation failed. See the error message(s) printed below. For more detailed information, check the log file from the workflow generation script: {logfile} diff --git a/ush/python_utils/config_parser.py b/ush/python_utils/config_parser.py index 7cae1b21f1..c1b69db5ff 100644 --- a/ush/python_utils/config_parser.py +++ b/ush/python_utils/config_parser.py @@ -44,11 +44,8 @@ def load_yaml_config(config_file): """Safe load a yaml file""" - try: - with open(config_file, "r") as f: - cfg = yaml.safe_load(f) - except yaml.YAMLError as e: - raise Exception(f"Unable to load yaml file {config_file}") + with open(config_file, "r") as f: + cfg = yaml.safe_load(f) return cfg diff --git a/ush/python_utils/print_msg.py b/ush/python_utils/print_msg.py index 9f38998c33..8db0f06b03 100644 --- a/ush/python_utils/print_msg.py +++ b/ush/python_utils/print_msg.py @@ -41,14 +41,14 @@ def print_info_msg(info_msg, verbose=True): return True return False -def log_info(info_msg, verbose=True, ddent=True): +def log_info(info_msg, verbose=True, dedent_=True): """Function to print information message using the logging module. This function should not be used if python logging has not been initialized. Args: info_msg : info message to print verbose : set to False to silence printing - ddent : set to False to disable "dedenting" (print string as-is) + dedent_ : set to False to disable "dedenting" (print string as-is) Returns: None """ @@ -57,7 +57,7 @@ def log_info(info_msg, verbose=True, ddent=True): logger=getLogger(sys._getframe().f_back.f_code.co_name) if verbose: - if ddent: + if dedent_: logger.info(indent(dedent(info_msg), ' ')) else: logger.info(info_msg) diff --git a/ush/set_ozone_param.py b/ush/set_ozone_param.py index 0bbbf06d47..5ed4449fe1 100644 --- a/ush/set_ozone_param.py +++ b/ush/set_ozone_param.py @@ -90,12 +90,8 @@ def set_ozone_param(ccpp_phys_suite_fp): fixgsm_ozone_fn = "global_o3prdlos.f77" ozone_param = "ozphys" else: - raise Exception( - f''' - Unknown or no ozone parameterization - specified in the CCPP physics suite file (ccpp_phys_suite_fp): - ccpp_phys_suite_fp = \"{ccpp_phys_suite_fp}\"''' - ) + raise KeyError(f'Unknown or no ozone parameterization specified in the ' + 'CCPP physics suite file "{ccpp_phys_suite_fp}"') # # ----------------------------------------------------------------------- # @@ -158,7 +154,7 @@ def set_ozone_param(ccpp_phys_suite_fp): """, verbose=VERBOSE) log_info(f""" CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING = {list_to_str(CYCLEDIR_LINKS_TO_FIXam_FILES_MAPPING)} - """, verbose=VERBOSE, ddent=False) + """, verbose=VERBOSE, dedent_=False) else: diff --git a/ush/setup.py b/ush/setup.py index 033180c760..5e125e3271 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -3,6 +3,7 @@ import os import sys import datetime +import traceback from textwrap import dedent from logging import getLogger @@ -88,8 +89,7 @@ def setup(): "FV3GFS_FILE_FMT_ICS", "FV3GFS_FILE_FMT_LBCS"]) - # Load the user config file, containing contains user-specified values - # variables that override their default values. Ensure all user-specified + # Load the user config file, then ensure all user-specified # variables correspond to a default value. if not os.path.exists(EXPT_CONFIG_FN): raise FileNotFoundError(f'User config file not found: EXPT_CONFIG_FN = {EXPT_CONFIG_FN}') @@ -99,16 +99,16 @@ def setup(): except: errmsg = dedent(f'''\n Could not load YAML config file: {EXPT_CONFIG_FN} - The file may be formatted incorrectly; reference the Users Guide for more info + Reference the above traceback for more information. ''') - raise Exception(errmsg) from None + raise Exception(errmsg) cfg_u = flatten_dict(cfg_u) for key in cfg_u: if key not in flatten_dict(cfg_d): raise Exception(dedent(f''' User-specified variable "{key}" in {EXPT_CONFIG_FN} is not valid - Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables\n''')) + Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables.\n''')) # Mandatory variables *must* be set in the user's config; the default value is invalid mandatory = ['MACHINE'] @@ -429,9 +429,10 @@ def get_location(xcs,fmt): # Mandatory variables *must* be set in the user's config or the machine file; the default value is invalid mandatory = ['NCORES_PER_NODE', 'FIXgsm', 'FIXaer', 'FIXlut', 'TOPO_DIR', 'SFC_CLIMO_INPUT_DIR'] + globalvars = globals() for val in mandatory: # globals() returns dictionary of global variables - if not globals()[val]: + if not globalvars[val]: raise Exception(dedent(f''' Mandatory variable "{val}" not found in: user config file {EXPT_CONFIG_FN} From d581228503dce9f3b6a73a1a9c6cd3f575125413 Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Fri, 21 Oct 2022 16:03:15 -0600 Subject: [PATCH 25/26] Comment to clarify use of manual "raise" in setup.py --- ush/setup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ush/setup.py b/ush/setup.py index 5e125e3271..65a46949ec 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -542,6 +542,7 @@ def get_location(xcs,fmt): # If using a custom post configuration file, make sure that it exists. if USE_CUSTOM_POST_CONFIG_FILE: try: + #os.path.exists returns exception if passed an empty string or None, so use "try/except" as a 2-for-1 error catch if not os.path.exists(CUSTOM_POST_CONFIG_FP): raise except: @@ -556,6 +557,7 @@ def get_location(xcs,fmt): # satellite products from the UPP, make sure the CRTM fix file directory exists. if USE_CRTM: try: + #os.path.exists returns exception if passed an empty string or None, so use "try/except" as a 2-for-1 error catch if not os.path.exists(CRTM_DIR): raise except: From d5259185114043287fe1e55546b193557ffbb6eb Mon Sep 17 00:00:00 2001 From: "Michael Kavulich, Jr" Date: Fri, 21 Oct 2022 16:04:20 -0600 Subject: [PATCH 26/26] One more comment addressed --- ush/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ush/setup.py b/ush/setup.py index 65a46949ec..3fa7313727 100644 --- a/ush/setup.py +++ b/ush/setup.py @@ -107,7 +107,7 @@ def setup(): for key in cfg_u: if key not in flatten_dict(cfg_d): raise Exception(dedent(f''' - User-specified variable "{key}" in {EXPT_CONFIG_FN} is not valid + User-specified variable "{key}" in {EXPT_CONFIG_FN} is not valid. Check {EXPT_DEFAULT_CONFIG_FN} for allowed user-specified variables.\n''')) # Mandatory variables *must* be set in the user's config; the default value is invalid