Skip to content

Add GCAFS forecast-only mode to the workflow #3606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

CoryMartin-NOAA
Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA commented Apr 21, 2025

Description

This PR adds in forecast-only mode for GCAFS and begins to put some of the pieces in place (mainly config files) for cycled mode for GCAFS/GCDAS. This PR also adds a lot of doc strings to places where they were missing. While I am opening this PR, a large portion of the work was done by @bbakernoaa

Partially addresses #3565

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? YES
  • Does this change require an update to any of the following submodules? NO
    • EMC verif-global
    • GDAS
    • GFS-utils
    • GSI
    • GSI-monitor
    • GSI-utils
    • UFS-utils
    • UFS-weather-model
    • wxflow

How has this been tested?

  • Clone and build on Hera
  • Forecast-only on Hera

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary

…s for snow, marine, and surface analysis tasks
…fig.tracker script for task-specific resources
@DavidHuber-NOAA DavidHuber-NOAA requested a review from Copilot April 30, 2025 16:35
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a forecast‐only mode for GCAFS into the global workflow while updating and adding comprehensive docstrings across multiple modules. Key changes include:

  • Adding new GCAFS-specific command-line arguments and configuration logic.
  • Registering new GCAFS task and XML generators in the Rocoto workflow.
  • Enhancing documentation and consistency in application configuration classes.

Reviewed Changes

Copilot reviewed 85 out of 90 changed files in this pull request and generated no comments.

Show a summary per file
File Description
dev/workflow/setup_expt.py Updated docstrings and added GCAFS forecast-only parser support
dev/workflow/rocoto/tasks_factory.py Registered GCAFSTasks for GCAFS
dev/workflow/rocoto/tasks.py Included GCAFS in forecast hours calculation
dev/workflow/rocoto/rocoto_xml_factory.py Registered GCAFSForecastOnlyRocotoXML
dev/workflow/rocoto/gfs_forecast_only_xml.py Docstring updates for GFS forecast-only XML generation
dev/workflow/rocoto/gfs_cycled_xml.py Docstring enhancements in cycled XML generator
dev/workflow/rocoto/gcafs_forecast_only_xml.py New module for generating GCAFS forecast-only cycle definitions
dev/workflow/applications/sfs.py Enhanced documentation for SFS application config
dev/workflow/applications/gfs_forecast_only.py Updated docstrings for GFS forecast-only configuration
dev/workflow/applications/gfs_cycled.py Improved documentation in GFS cycled configuration
dev/workflow/applications/gefs.py Enhanced GEFS configuration docstrings
dev/workflow/applications/gcafs_forecast_only.py New GCAFS forecast-only app config with configuration adjustments
dev/workflow/applications/applications.py Updated abstract base and initialization docstrings
dev/workflow/applications/application_factory.py Registered GCAFS forecast-only app configuration
dev/ci/cases/yamls/gcafs_defaults_ci.yaml Added CI YAML file for GCAFS default configuration
Files not reviewed (5)
  • docs/source/development.rst: Language not supported
  • docs/source/gcafs.rst: Language not supported
  • docs/source/index.rst: Language not supported
  • jobs/JGLOBAL_OFFLINE_ATMOS_ANALYSIS: Language not supported
  • parm/chem/chem_emission.yaml.j2: Language not supported
Comments suppressed due to low confidence (2)

dev/workflow/applications/gcafs_forecast_only.py:11

  • The logic for setting app_config.configs['base']['RUN'] to 'gcafs' appears redundant with multiple if/else branches; consider simplifying this block to improve clarity.
if 'base' in app_config.configs:

dev/workflow/applications/gcafs_forecast_only.py:139

  • [nitpick] There is commented-out code in the get_task_names method; if it is no longer needed, please remove it to reduce clutter.
// if options['do_archcom']:
//     tasks += ['arch_tars']
//     if options['do_globusarch']:
//         tasks += ['globus_arch']

Copy link
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.

I think a few of the parm/config/gcafs files should be links rather than standalone. Otherwise, I think this is pretty close.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a link?

@CoryMartin-NOAA
Copy link
Contributor Author

Anything else to change/fix before kicking off testing?

Copy link
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.

Looks good. One question about future testing.

@DavidHuber-NOAA
Copy link
Contributor

Starting CI on C6.

@DavidHuber-NOAA DavidHuber-NOAA added the CI-Gaeac6-Ready **CM use only** PR is ready for CI testing on Gaea C6 label May 13, 2025
@emcbot emcbot added CI-Gaeac6-Building **Bot use only** CI testing is cloning/building on Gaea C6 CI-Gaeac6-Running CI-Gaeac6-Passed **Bot use only** CI testing on Gaea C6 for this PR has completed successfully and removed CI-Gaeac6-Ready **CM use only** PR is ready for CI testing on Gaea C6 CI-Gaeac6-Building **Bot use only** CI testing is cloning/building on Gaea C6 CI-Gaeac6-Running labels May 13, 2025
@DavidHuber-NOAA
Copy link
Contributor

@CoryMartin-NOAA All tests passed on C6. Could you please resolve the conflicts in your branch? Also, you may need to move parm/config/gcafs/* to dev/parm/config/gcafs. Once complete, I think this PR is ready to merge.

@CoryMartin-NOAA
Copy link
Contributor Author

Done. Thanks @DavidHuber-NOAA

@DavidHuber-NOAA DavidHuber-NOAA merged commit d9987d8 into NOAA-EMC:develop May 14, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-Gaeac6-Passed **Bot use only** CI testing on Gaea C6 for this PR has completed successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants