search

ufs-community / ufs-srweather-app

UFS Short-Range Weather Application
Other
55 stars 116 forks source link

Should "EXPT_SUBDIR" be a mandatory variable? #978

Closed mkavulich closed 8 months ago

mkavulich commented 9 months ago

Description

Currently, the Users Guide (as well as in-line documentation in config_defaults.yaml) states that EXPT_SUBDIR is a mandatory variable: it must be specified by the user. This seems draconian to me, as the only thing it is used for is setting the subdirectory where an experiment is run. And what's more, if the user specifies their own custom full-path value for EXPTDIR, it doesn't get used at all!

As an additional headache, there is currently a bug in the workflow generation that fails in a very messy and unintuitive way if EXPT_SUBDIR is not set:

ln: target ‘}}/launch_FV3LAM_wflow.sh’ is not a directory
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/./generate_FV3LAM_wflow.py", line 768, in <module>
    expt_dir = generate_FV3LAM_wflow(USHdir, wflow_logfile, pargs.debug)
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/./generate_FV3LAM_wflow.py", line 143, in generate_FV3LAM_wflow
    create_symlink_to_file(
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/create_symlink_to_file.py", line 53, in create_symlink_to_file
    ln_vrfy(f"-sf {relative_flag} {target} {symlink}")
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/filesys_cmds_vrfy.py", line 41, in ln_vrfy
    return cmd_vrfy("ln", *args)
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/filesys_cmds_vrfy.py", line 20, in cmd_vrfy
    print_err_msg_exit(f"System call '{cmd}' failed.")
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/print_msg.py", line 20, in print_err_msg_exit
    traceback.print_stack(file=sys.stderr)
FATAL ERROR: System call 'ln -sf  /scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/launch_FV3LAM_wflow.sh /scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/expt_dirs/{{ EXPT_SUBDIR }}/launch_FV3LAM_wflow.sh' failed.
Exiting with nonzero status.

*********************************************************************
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: /scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/log.generate_FV3LAM_wflow
*********************************************************************

Traceback (most recent call last):
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/./generate_FV3LAM_wflow.py", line 768, in <module>
    expt_dir = generate_FV3LAM_wflow(USHdir, wflow_logfile, pargs.debug)
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/./generate_FV3LAM_wflow.py", line 143, in generate_FV3LAM_wflow
    create_symlink_to_file(
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/create_symlink_to_file.py", line 53, in create_symlink_to_file
    ln_vrfy(f"-sf {relative_flag} {target} {symlink}")
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/filesys_cmds_vrfy.py", line 41, in ln_vrfy
    return cmd_vrfy("ln", *args)
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/filesys_cmds_vrfy.py", line 20, in cmd_vrfy
    print_err_msg_exit(f"System call '{cmd}' failed.")
  File "/scratch1/BMC/hmtb/kavulich/UFS/workdir/issue_966/ufs-srweather-app/ush/python_utils/print_msg.py", line 24, in print_err_msg_exit
    sys.exit(1)
SystemExit: 1

Solution

One of two things should be changed here:

  1. EXPT_SUBDIR should get a default value. The literal string "experiment" seems appropriate, but it could really be anything so long as it's documented appropriately.
  2. generate_FV3LAM_wflow() should fail with a verbose/explicit error message if EXPT_SUBDIR is not specified. This seems as if it was intended behavior, but was not implemented correctly: there is a check for empty EXPT_SUBDIR in setup.py, but it is not empty if not specified in config.yaml, but rather gets set to the literal string {{ EXPT_SUBDIR }}.

I prefer the first solution, but either would be acceptable.

Requirements**

Users should not receive strange errors if they do not set set EXPT_SUBDIR.

Acceptance Criteria (Definition of Done)

Implement either one of the suggested solutions above