Skip to content

Switch to using C++ IMS SCF preprocessor#4000

Merged
DavidHuber-NOAA merged 9 commits into
NOAA-EMC:developfrom
CoryMartin-NOAA:feature/ims-scf-cpp
Sep 5, 2025
Merged

Switch to using C++ IMS SCF preprocessor#4000
DavidHuber-NOAA merged 9 commits into
NOAA-EMC:developfrom
CoryMartin-NOAA:feature/ims-scf-cpp

Conversation

@CoryMartin-NOAA
Copy link
Copy Markdown
Contributor

@CoryMartin-NOAA CoryMartin-NOAA commented Sep 3, 2025

Description

This PR switches the snow_analysis and snowens_analysis tasks from using the Fortran + Python IMS SCF preprocessing workflow to a C++ OOPS executable.

Closes #3980

First step towards #3933

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? NO
  • Does this change require an update to any of the following submodules? YES
    • EMC verif-global
    • GDAS - Already merged
    • GFS-utils
    • GSI
    • GSI-monitor
    • GSI-utils
    • UFS-utils
    • UFS-weather-model
    • wxflow

How has this been tested?

  • Cycled test on GaeaC6

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

@CoryMartin-NOAA CoryMartin-NOAA self-assigned this Sep 3, 2025
@CoryMartin-NOAA CoryMartin-NOAA added JEDI Feature development to support JEDI-based DA GFS labels Sep 3, 2025
aerorahul
aerorahul previously approved these changes Sep 3, 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.

lgtm.
I haven't paid much attention here, but it seems to me that most methods in snow_analysis.py and snowens_analysis.py are repetitive, and could be combined/consolidated to reduce burden.

@CoryMartin-NOAA
Copy link
Copy Markdown
Contributor Author

@aerorahul agreed, in the current configuration. If we do EDA or EnKF for snow in the future, would be better to have them remain separated though.

jiaruidong2017
jiaruidong2017 previously approved these changes Sep 3, 2025
Comment thread ush/python/pygfs/task/snow_analysis.py
Comment thread ush/python/pygfs/task/snowens_analysis.py
@ClaraDraper-NOAA
Copy link
Copy Markdown
Contributor

@aerorahul agreed, in the current configuration. If we do EDA or EnKF for snow in the future, would be better to have them remain separated though.

@ClaraDraper-NOAA
Copy link
Copy Markdown
Contributor

ClaraDraper-NOAA commented Sep 3, 2025

For people's awareness:

Note that I'm working on updating the GHCN snow depth obs to the global_workflow. The GHCN IODA converter will need a similar treatment.

Additionally, I'm also working on a simplification to the snow_analysis task, which will result in moving the prep_IMS call (and some changes in the paths).

Copy link
Copy Markdown
Contributor

@DavidNew-NOAA DavidNew-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. Thanks for the changes

Copy link
Copy Markdown
Contributor

@ClaraDraper-NOAA ClaraDraper-NOAA left a comment

Choose a reason for hiding this comment

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

lgtm

@DavidNew-NOAA
Copy link
Copy Markdown
Contributor

@aerorahul @DavidHuber-NOAA If there are no objections, can we get CI going for this PR? I have a PR that needs to integrate the changes from this one to move forward.

@aerorahul
Copy link
Copy Markdown
Contributor

No objections from me. Please go ahead.

@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

Launching tests on C6.

@emcbot emcbot added 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 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 labels Sep 4, 2025
@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

I accidentally killed the GitLab pipeline. I'll finish running the tests manually on C6.

@DavidNew-NOAA
Copy link
Copy Markdown
Contributor

CI passes on Hera

******** C48_ATM ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202103231200        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:54:04
******** C48mx500_3DVarAOWCDA ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202103241800        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:14:04
202103250000        Done    Sep 04 2025 18:55:13    Sep 04 2025 20:44:09
******** C48mx500_hybAOWCDA ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202103241800        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:14:04
202103250000        Done    Sep 04 2025 18:55:13    Sep 04 2025 20:04:03
******** C48_S2SWA_gefs ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202103231200        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:56:04
******** C48_S2SW ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202103231200        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:52:04
******** C96_atm3DVar ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202112201800        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:12:05
202112210000        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:16:03
202112210600        Done    Sep 04 2025 18:55:13    Sep 04 2025 20:52:07
******** C96C48_hybatmDA ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202112201800        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:14:04
202112210000        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:14:04
202112210600        Done    Sep 04 2025 18:55:13    Sep 04 2025 20:42:06
******** C96C48_hybatmsnowDA ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202112201200        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:12:04
202112201800        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:08:03
202112210000        Done    Sep 04 2025 18:55:13    Sep 04 2025 20:42:06
******** C96C48_hybatmsoilDA ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202205150600        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:14:04
202205151200        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:24:05
202205151800        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:10:12
******** C96C48mx500_S2SW_cyc_gfs ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202112201200        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:14:04
202112201800        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:20:12
202112210000        Done    Sep 04 2025 18:55:13    Sep 04 2025 22:02:04
202112211800        Done    Sep 04 2025 19:16:04    Sep 04 2025 21:58:04
******** C96C48_ufs_hybatmDA ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202402231800        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:14:04
202402240000        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:30:14
202402240600        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:02:04
******** C96_gcafs_cycled ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202112201200        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:14:04
202112201800        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:14:04
202112210000        Done    Sep 04 2025 18:55:13    Sep 04 2025 20:04:03
******** C96_gcafs_cycled_noDA ********
   CYCLE         STATE           ACTIVATED              DEACTIVATED     
202112201200        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:12:04
202112201800        Done    Sep 04 2025 18:55:13    Sep 04 2025 21:06:04
202112210000        Done    Sep 04 2025 18:55:13    Sep 04 2025 19:52:05

@TerrenceMcGuinness-NOAA
Copy link
Copy Markdown
Collaborator

TerrenceMcGuinness-NOAA commented Sep 4, 2025

@DavidHuber-NOAA I restarted the Pipeline. Do not update the labels.
@DavidNew-NOAA Nice formatting on your self test. We can add that to our output if you prefer.

Oh they finished right off. So you guys finished the cases in place?
/gpfs/f6/drsa-precip3/world-shared/global/CI/GITLAB/pr_cases_4000_8a9ee157_3963/RUNTESTS/EXPDIR

@emcbot emcbot added CI-Gaeac6-Passed **Bot use only** CI testing on Gaea C6 for this PR has completed successfully and removed CI-Gaeac6-Running labels Sep 4, 2025
@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

Oh they finished right off. So you guys finished the cases in place? /gpfs/f6/drsa-precip3/world-shared/global/CI/GITLAB/pr_cases_4000_8a9ee157_3963/RUNTESTS/EXPDIR

Yes: #4000 (comment)

@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

Thanks for relaunching the pipeline @TerrenceMcGuinness-NOAA. I forgot we can do that now.

@DavidHuber-NOAA DavidHuber-NOAA added the CI-Hera-Passed **Bot use only** CI testing on Hera for this PR has completed successfully label Sep 5, 2025
@DavidHuber-NOAA
Copy link
Copy Markdown
Contributor

All tests passed. Merging.

@DavidHuber-NOAA DavidHuber-NOAA merged commit cf34da5 into NOAA-EMC:develop Sep 5, 2025
5 checks passed
DavidHuber-NOAA pushed a commit that referenced this pull request Sep 29, 2025
PR #4000 switched to using the C++ IMS code and in that we changed how
that part of the snow analysis job is called, in this, I introduced a
bug that would arise if the IMS input file was missing. This fixes this.

  Resolves #3980
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 CI-Hera-Passed **Bot use only** CI testing on Hera for this PR has completed successfully GFS JEDI Feature development to support JEDI-based DA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modify snow analysis jobs to use new C++ IMS SCF preprocessor

8 participants