Addressing several issues: bug fixes and improvements by mkavulich · Pull Request #25 · dtcenter/dtc-vx-workflow

mkavulich · 2026-06-23T16:38:08Z

DESCRIPTION OF CHANGES:

This PR resolves a number of open issues in the repository:

Resolved issues:

Need to be able to pull TC best track data from external source #6 The TCPAIRS task has been updated to include logic to automatically pull best track data from the NHC FTP server.
Need to update PR and Issue templates #7 The PR and issue templates are updated to be more relevant to this repository
--launch cron option for ./run_we2e_tests.py is broken #18 The capability of submitting rocoto tasks from cron was broken at some point in the early stages of this repository. The fix involves a few improvements/fixes:
- The get_crontab_contents() and add_crontab_line() functions were not updated properly when debug flags were re-configured in b92b0e2
- Use setup_conda.sh to set up the python environment for cron submission
- Since we no longer have module loads, ROCOTO_PATH is provided as a variable in the machine file for each platform in the cron submission scripts
Need setup_conda.sh to handle user-maintained conda environment #22 The setup_conda.sh script was re-written to allow the user to use a conda environment in a custom location. This involved a few improvements/fixes:
- The case where LD_LIBRARY_PATH is unset is now handled correctly
- Fixes to allow the setup_conda.sh script to work from any directory.
- The confusing "conda activate" message after conda was already activated is now suppressed.
PCPCombine forecast tasks have incorrect check for obs dir #23 Fixed incorrect check for OBS_DIR for PCP combine Forecast tasks
NOHRSC obs retrieval tasks fail when pulling from HPSS #24 Fixed bad f-string in get_obs.py for NOHRSC observations.

Other changes:

Staged data is now present on Hera/Ursa so that all WE2E tests will pass. Future work will be needed for other platforms that do not have access to HPSS (see Need capability to automatically pull observations from sources other than HPSS #5)
The retrieve_data.py CI test is disabled for now; this needs to be updated in order to test our actual use cases (CI test for retrieve_data.py needs to be updated to test observation pulling #26)
With the fix for launching jobs from cron, the system is now set up to allow for a platform-specific ROCOTO_PATH that skips the need to load rocoto modules on those machines

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
- As part of the capability to pull best track data from FTP, the BEST_TRACK variable is renamed to BEST_TRACK_DIR and BEST_TRACK_FILE is now an option to specify the best track filename rather than having it hard-coded in conf files. There is now a check in setup.py to ensure that old user config files will fail with a descriptive error message.

TESTS CONDUCTED:

Ran all WE2E tests on Ursa. All pass now!

DEPENDENCIES:

None

DOCUMENTATION:

None needed

ISSUE:

Fixes:

CONTRIBUTORS (optional):

A few commits in this PR were authored, partially or in full, by Claude CLI. These commits include the specific model used as a co-author. All of these changes were manually reviewed by me and checked for correctness.

- Replace call to load_modules_wflow.sh with call to setup_conda.sh - Since we no longer have module loads, ROCOTO_PATH is provided as a variable in the machine file for each platform

…king yet

… does this automatically

Read ROCOTO_PATH from the machine config file and prefix all calls to rocotorun and rocotostat with it, consistent with launch_vx_wflow.sh. Machines without ROCOTO_PATH (jet, noaacloud, linux, macos) fall back to bare binary names. rocoto_path is stored in the WE2E monitor YAML and propagated through update_expt_status/compare_rocotostat; added to all non-task entry skip lists to avoid being treated as a task. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The statuses loop in update_expt_status had 8-space indentation while the other two skip lists had 12-space indentation, so the replace_all edit missed it. This caused a TypeError when iterating over the expt dict since rocoto_path (a string) was not excluded from the task loop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Resolve SCRIPT_DIR from ${BASH_SOURCE[0]} at the top and anchor all relative file/directory references (conda_loc, conda/, environment.yml) to it. Also replaces readlink -f with cd && pwd for macOS compatibility without requiring GNU coreutils. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

A user-defined cd function in .bashrc that prints to stdout would corrupt any variable set via $(cd ... && pwd) command substitution. Using builtin cd bypasses shell functions and calls the built-in directly, preventing spurious output from being captured. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…! Also, make HAFS-A WE2E test case work on all platforms

…observations rather than model data

…ng the rocoto exes

…ent <native> tag for rocoto when submitting to the HPSS queue (just like how get_obs tasks already do)

gsketefian · 2026-07-08T15:24:57Z

@mkavulich FYI I'm just starting my review.

gsketefian · 2026-07-08T15:33:54Z


 ## TESTS CONDUCTED: 
-<!-- Explicitly state what tests were run on these changes, or if any are still pending (for README or other text-only changes, just put "None required"). Make note of the compilers used, the platform/machine, and other relevant details as necessary. For more complicated changes, or those resulting in scientific changes, please be explicit! -->
+<!-- Explicitly state what tests were run on these changes, or if any are still pending (for README or other text-only changes, just put "None required"). Make note of the platform/machine, and other relevant details as necessary. For more complicated changes, or those resulting in scientific changes, please be explicit! -->


Remove comma after "platform/machine" (according to my middle school grammar lessons).

gsketefian · 2026-07-08T17:32:23Z

+# (conda_loc, conda/, environment.yml) work regardless of the caller's CWD.
+SCRIPT_DIR=$(builtin cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+
+# Check for existing conda/ subdirectory from previous installations


This comment would be clearer if changed to something like this:

# If the conda location file is not specified but a conda installation exists under dtc-vx-workflow from a previous installation...

Updated the comment with more detail

gsketefian · 2026-07-08T17:32:28Z

-if ! conda env list | grep -q "^vx_workflow\s" ; then
-  mamba env create -n vx_workflow --file "${VX_WFLOW_DIR}/environment.yml" -y
+
+# Check if conda location file exists


This comment would be clearer if changed to something like this:

# If the conda location file is not specified but a (system) conda installation is still available...

Updated for a more clear and accurate comment

mkavulich

@gsketefian Thanks for your review, I believe I've addressed all your comments

mkavulich · 2026-07-08T20:37:52Z


 ## TESTS CONDUCTED: 
-<!-- Explicitly state what tests were run on these changes, or if any are still pending (for README or other text-only changes, just put "None required"). Make note of the compilers used, the platform/machine, and other relevant details as necessary. For more complicated changes, or those resulting in scientific changes, please be explicit! -->
+<!-- Explicitly state what tests were run on these changes, or if any are still pending (for README or other text-only changes, just put "None required"). Make note of the platform/machine, and other relevant details as necessary. For more complicated changes, or those resulting in scientific changes, please be explicit! -->


mkavulich · 2026-07-09T15:27:54Z

+# (conda_loc, conda/, environment.yml) work regardless of the caller's CWD.
+SCRIPT_DIR=$(builtin cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+
+# Check for existing conda/ subdirectory from previous installations


Updated the comment with more detail

mkavulich · 2026-07-09T15:29:39Z

-if ! conda env list | grep -q "^vx_workflow\s" ; then
-  mamba env create -n vx_workflow --file "${VX_WFLOW_DIR}/environment.yml" -y
+
+# Check if conda location file exists


Updated for a more clear and accurate comment

gsketefian · 2026-07-09T16:27:37Z

@mkavulich Left a couple of replies and approved.

…ng the rocoto exes

mkavulich and others added 20 commits June 16, 2026 13:01

Add staged location data for Ursa

dfc5cf7

Fix issue with launching jobs from cron

f079f0f

Fix setup_conda for environments where LD_LIBRARY_PATH is unset

894de82

Fix issues with crontab submission:

4514341

- Replace call to load_modules_wflow.sh with call to setup_conda.sh - Since we no longer have module loads, ROCOTO_PATH is provided as a variable in the machine file for each platform

Attempting to get automated pulling of best track data; not quite wor…

8f6b232

…king yet

borrow latest conda setup logic from mpas_plot repository

76567bc

Wording fixes, suppress "conda activate" message since setup_conda.sh…

abae915

… does this automatically

Pulling of A-DECK files now working!

030d0b6

I'm dumb, we need to retrieve BDECK, not ADECK files. Working now!

43e9d90

Need to unzip retrieved file

b003127

Fix some commands and templates, best track pulling now fully working…

a0dbfaa

…! Also, make HAFS-A WE2E test case work on all platforms

Update PR and Issue templates

e548648

Dont overwrite conda_loc every time setup_conda.sh is invoked

1c48fdf

Add check in setup.py for old BEST_TRACK variable name

ac52932

Fix unformatted f-string in get_obs.py

a53fb40

Fix incorrect check for OBS_DIR in pcpcombine.sh for FCST tasks

6981d8f

mkavulich requested review from EdwardSnyder-NOAA, JeffBeck-NOAA and gsketefian as code owners June 23, 2026 16:38

mkavulich added 7 commits June 23, 2026 16:48

Fix test for get_crontab_contents.py

96241ed

Lint tcpairs.py

66f6083

Not running test_retrieve_data.py for now; need to adapt to retrieve …

a1aba00

…observations rather than model data

Fix pylint check

56d3d1c

Restore fix for setup_conda.sh when LD_LIBRARY_PATH is unset

982c943

ACTUALLY fix pylint check

2d6cd5a

Completely disable retrieve_data.py test

0666859

Merge branch 'develop' into feature/fix_some_issues

09e095b

georgemccabe mentioned this pull request Jun 30, 2026

[develop]: Add regression testing #28

Merged

34 tasks

georgemccabe added a commit that referenced this pull request Jun 30, 2026

remove module load of rocoto since the changes in PR #25 handle findi…

7c32a83

…ng the rocoto exes

Fix error submitting TCPAIRS job on Hera: we need to specify a differ…

69228c0

…ent <native> tag for rocoto when submitting to the HPSS queue (just like how get_obs tasks already do)