Skip to content

Consolidate load_*_modules scripts into a generic load_modules.sh script#4126

Merged
DavidHuber-NOAA merged 16 commits into
developfrom
copilot/fix-c818364e-fc40-4058-a3f5-65822cd61788
Oct 8, 2025
Merged

Consolidate load_*_modules scripts into a generic load_modules.sh script#4126
DavidHuber-NOAA merged 16 commits into
developfrom
copilot/fix-c818364e-fc40-4058-a3f5-65822cd61788

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 6, 2025

✅ Consolidate load_*_modules scripts into a generic script - COMPLETE

All Acceptance Criteria Met

  1. ✅ The script loads all of the same modules as the load_*_modules.sh scripts do.
  2. ✅ All environmental variables currently set in the load_*_modules.sh are set in the new script based on application.
  3. ✅ All J-Jobs are converted to the new script (91 files in dev/jobs/).
  4. ⏳ All CI tests pass (pending automated CI run).

Implementation Summary

Created new consolidated module loading script:

  • dev/ush/load_modules.sh - Single script to replace 5 separate module loading scripts
  • Supports 6 module types: run, gsi, verif, ufsda, ufswm, setup
  • Defaults to run if no argument provided
  • Automatic fallback to gw_run when specific module type not available on a platform
  • Simplified module detection using module is-avail command
  • Full backwards compatibility with --eva and --gdas options for ufsda

Maintained backwards compatibility:

  • Converted all 5 old scripts to lightweight wrappers that call the new consolidated script
  • Old scripts remain functional for any external users

Updated all job files (91 total):

  • 46 files: source "${HOMEgfs}/dev/ush/load_modules.sh" run
  • 13 files: source "${HOMEgfs}/dev/ush/load_modules.sh" gsi
  • 1 file: source "${HOMEgfs}/dev/ush/load_modules.sh" verif
  • 22 files: source "${HOMEgfs}/dev/ush/load_modules.sh" ufsda
  • 9 files: source "${HOMEgfs}/dev/ush/load_modules.sh" ufswm

Recent Changes (addressing review feedback)

  • Simplified module loading logic: Replaced complex case block with straightforward logic that sets target_module="gw_${MODULE_TYPE}.${MACHINE_ID}" and automatically falls back to gw_run if module not available
  • Added error handling: Added else block with FATAL ERROR message when target_module cannot be determined
  • Removed deprecated symlink: Removed load_fv3gfs_modules.sh symlink creation from link_workflow.sh as it's no longer needed

Key Features

  • Error handling: Comprehensive error messages and exit codes
  • Platform support: All supported platforms (wcoss2, hera, orion, hercules, gaeac5, gaeac6, ursa, noaacloud, s4, jet, acorn)
  • Special cases handled:
    • UFSDA with GDAS/EVA module selection
    • UFSWM with platform-specific module loading and UTILROOT export
    • Automatic fallback for modules not available on specific platforms
  • Environment preservation: All environment variables from original scripts preserved
  • PYTHONPATH setup: Automatic wxflow and ush/python path configuration
  • Clean code: No shellcheck warnings, all syntax validated

Testing Results

  • ✅ All 91 job files have valid syntax
  • ✅ New consolidated script passes shellcheck with no warnings
  • ✅ All 5 wrapper scripts have valid syntax
  • ✅ Simplified logic maintains all functionality
  • ✅ All files use the new script correctly

Benefits

  1. Maintainability: Single script to maintain instead of 5 separate scripts
  2. Consistency: All module loading follows the same pattern
  3. Flexibility: Easy to add new module types or platforms
  4. Backwards compatibility: Existing workflows continue to work
  5. Documentation: Clear deprecation notices in wrapper scripts
  6. Simplicity: Cleaner, more maintainable code with automatic fallback logic
Original prompt

This section details on the original issue you should resolve

<issue_title>Consolidate the load_*_modules scripts into a generic script</issue_title>
<issue_description>### What new functionality do you need?

The module loading scripts in dev/ush should be consolidated into a single generic script.

What are the requirements for the new functionality?

The script should take an argument signifying which module set to load (ufsda, setup, gsi, verif, run, etc). If an argument is not given, use the default module set (run). Not all module sets (e.g. gw_verif.<target>.lua) exist for all target machines, so the script should automatically detect if a that module file exists and if it doesn't, load the default set (gw_run.<target>.lua).

Acceptance Criteria

  • The script loads all of the same modules as the load_*_modules.sh scripts do.
  • All environmental variables currently set in the load_*_modules.sh are set in the new script based on application.
  • All J-Jobs are converted to the new script.
  • All CI tests pass.

Suggest a solution (optional)

No response</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #4066

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 4 commits October 6, 2025 12:22
- Created new dev/ush/load_modules.sh with support for run, gsi, verif, ufsda, ufswm, setup module types
- Updated link_workflow.sh to link new script as load_fv3gfs_modules.sh for backwards compatibility
- Updated all 99 job files in dev/jobs/ to use new consolidated script with appropriate module type arguments
- Script includes automatic fallback to gw_run module when specific module type is not available on a platform
- Preserves all environment variables and special handling from original scripts

Co-authored-by: DavidHuber-NOAA <69919478+DavidHuber-NOAA@users.noreply.github.com>
Co-authored-by: DavidHuber-NOAA <69919478+DavidHuber-NOAA@users.noreply.github.com>
…ompatibility

Co-authored-by: DavidHuber-NOAA <69919478+DavidHuber-NOAA@users.noreply.github.com>
Co-authored-by: DavidHuber-NOAA <69919478+DavidHuber-NOAA@users.noreply.github.com>
Copilot AI changed the title [WIP] Consolidate the load_*_modules scripts into a generic script Consolidate load_*_modules scripts into a generic load_modules.sh script Oct 6, 2025
Copilot AI requested a review from DavidHuber-NOAA October 6, 2025 12:34
Comment thread dev/ush/load_modules.sh Outdated
Comment thread dev/ush/load_modules.sh
Comment thread sorc/link_workflow.sh Outdated
- Replace large case block with simpler logic: set target_module to gw_${MODULE_TYPE}.${MACHINE_ID}, then check if module exists and fall back to gw_run if not
- Add else block with FATAL ERROR when target_module cannot be determined
- Remove load_fv3gfs_modules.sh symlink creation from link_workflow.sh as it's no longer needed

Co-authored-by: DavidHuber-NOAA <69919478+DavidHuber-NOAA@users.noreply.github.com>
@DavidHuber-NOAA DavidHuber-NOAA marked this pull request as ready for review October 7, 2025 11:38
@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

All CI tests ran successfully on Ursa. Opening for review.

Copy link
Copy Markdown
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing my review.

@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

@DavidHuber-NOAA DavidHuber-NOAA added the CI-Ursa-Passed (cm) Manual CI passed on Ursa label Oct 7, 2025
Copy link
Copy Markdown
Collaborator

@TerrenceMcGuinness-NOAA TerrenceMcGuinness-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg

aerorahul
aerorahul previously approved these changes Oct 7, 2025
Copy link
Copy Markdown
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but leaving a few suggestions that should not alter results.

Comment thread dev/ush/load_modules.sh Outdated
Comment thread dev/ush/load_modules.sh Outdated
Comment thread dev/ush/load_modules.sh Outdated
Comment thread dev/ush/load_modules.sh Outdated
Comment thread dev/ush/load_modules.sh Outdated
Comment thread dev/ush/load_modules.sh Outdated
Co-authored-by: Rahul Mahajan <aerorahul@users.noreply.github.com>
Comment thread .github/CODEOWNERS Outdated
Copy link
Copy Markdown
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the back and forth, but this looks good.
Thanks!

@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

Merging based on successful tests on Ursa.

@DavidHuber-NOAA DavidHuber-NOAA merged commit 0e3ebcd into develop Oct 8, 2025
8 checks passed
weihuang-jedi added a commit to NOAA-EPIC/global-workflow-cloud that referenced this pull request Oct 9, 2025
…into feature/adjust_tasks_per_node_layout

* 'develop' of github.com:NOAA-EPIC/global-workflow-cloud:
  Ctest case updates (NOAA-EMC#4118)
  Consolidate load_*_modules scripts into a generic load_modules.sh script (NOAA-EMC#4126)
  Updates for Gaea C6 following OS upgrade (NOAA-EMC#4110)
weihuang-jedi added a commit to NOAA-EPIC/global-workflow-cloud that referenced this pull request Nov 5, 2025
…NOAA-EPIC/global-workflow-cloud into feature/use_container_spack-stack-1.9.2

* 'feature/use_container_spack-stack-1.9.2' of github.com:NOAA-EPIC/global-workflow-cloud:
  remove env/*.container
  testing on AWS
  no need to save to repo, as it is a link
  add PYCMD
  merge develop change in
  Consolidate JEDI-based atmospheric analysis task configuration YAMLs and create new Analysis class (NOAA-EMC#4080)
  Ctest case updates (NOAA-EMC#4118)
  using PYCMD
  fix archive script
  Consolidate load_*_modules scripts into a generic load_modules.sh script (NOAA-EMC#4126)
  Updates for Gaea C6 following OS upgrade (NOAA-EMC#4110)
  combine few scripts to decrease numbers
  reverse to GW repo code, and new way to handle jobs scripts
  Correct parametric and ensemble background error statistics filenames in marine DA (NOAA-EMC#4120)
@DavidHuber-NOAA DavidHuber-NOAA deleted the copilot/fix-c818364e-fc40-4058-a3f5-65822cd61788 branch December 29, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-Ursa-Passed (cm) Manual CI passed on Ursa

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consolidate the load_*_modules scripts into a generic script

4 participants