Skip to content

Marine DA script hardening #3797

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 15 commits into
base: develop
Choose a base branch
from
Draft

Marine DA script hardening #3797

wants to merge 15 commits into from

Conversation

DavidNew-NOAA
Copy link
Contributor

@DavidNew-NOAA DavidNew-NOAA commented Jun 13, 2025 •
edited
Loading

Description

This PR hardens the JEDI marine analysis code by ensuring that if the analysis observation statistics applications fails to initialize, run, and produce the correct output, the gdas_marineanlinit, gdas_marineanlchkpt, and gdas_marineanlfinal jobs won't fail and crash the workflow. My testing indicates that I can leave the archive jobs alone without them failing under this scenario.

I've also updated the saving-to-com for JEDI marine and atmospheric DA to save to COM_CONF. This required some updates to archiving.

Resolves NOAA-EMC/repo#5678

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)? YES
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? YES

How has this been tested?

  • Clone and build on Hera.
  • Run C96C48_ufs_hybatmDA, C96C48_hybatmDA, C48mx500_3DVarAOWCDA, C48mx500_hybAOWCDA, and C96C48_hybatmsnowDA

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

This was referenced Jun 13, 2025
Open
Merged
@DavidNew-NOAA
Copy link
Contributor Author

I've tested this in one round, but will do more comprehensive testing now that I'm mostly finished

@DavidNew-NOAA DavidNew-NOAA changed the title Feature/marine harden Marine DA script hardening Jun 13, 2025
guillaumevernieres
guillaumevernieres previously approved these changes Jun 13, 2025
View reviewed changes
Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turn around @DavidNew-NOAA

DavidNew-NOAA added a commit to NOAA-EMC/jcb-gdas that referenced this pull request Jun 13, 2025
@DavidNew-NOAA
Save soca_diag_stats yaml with JCB (#141)
5162a88 
This PR moves the saving of the soca_diag_stats yaml into the JCB
template that already saves the obs statistics.

Issue: NOAA-EMC/GDASApp#1738
Companions PRs: 
NOAA-EMC/GDASApp#1748 
NOAA-EMC/global-workflow#3797
@DavidNew-NOAA
Copy link
Contributor Author

I'm going on leave this week, so I'll just leave this note here so I don't forgot. Now that this PR saves marine and atmospheric YAMLs to COMOUT_CONF, the archiving job fails. Probably an easy fix. All other jobs are successful though.

@RussTreadon-NOAA RussTreadon-NOAA mentioned this pull request Jun 18, 2025
Closed
@DavidNew-NOAA
Copy link
Contributor Author

@CoryMartin-NOAA @guillaumevernieres @AndrewEichmann-NOAA @RussTreadon-NOAA @aerorahul My companion PR for GDASApp hasn't merged yet, but I've tested this PR thoroughly C96C48_ufs_hybatmDA, C96C48_hybatmDA, C48mx500_3DVarAOWCDA, C48mx500_hybAOWCDA, and C96C48_hybatmsnowDA cases. Looking for any final comments.

aerorahul
aerorahul reviewed Jun 26, 2025
View reviewed changes
Copy link
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.

some feedback

scripts/exglobal_marine_analysis_checkpoint.py
Comment on lines +28 to +31
try:
MarineAnl.execute('soca_diag_stats')
except Exception as e:
logger.warning(f"WARNING: Execution of soca_diag_stat application failed: {e}")
Copy link
Contributor

Choose a reason for hiding this comment

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

execute raises a Workflow exception, and the program will terminate. I wonder if the try-except is relevant or needed here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aerorahul aerorahul aerorahul left review comments

@guillaumevernieres guillaumevernieres guillaumevernieres left review comments

@CoryMartin-NOAA CoryMartin-NOAA Awaiting requested review from CoryMartin-NOAA CoryMartin-NOAA is a code owner

@AndrewEichmann-NOAA AndrewEichmann-NOAA Awaiting requested review from AndrewEichmann-NOAA AndrewEichmann-NOAA is a code owner

@RussTreadon-NOAA RussTreadon-NOAA Awaiting requested review from RussTreadon-NOAA RussTreadon-NOAA will be requested when the pull request is marked ready for review RussTreadon-NOAA is a code owner

@DavidHuber-NOAA DavidHuber-NOAA Awaiting requested review from DavidHuber-NOAA DavidHuber-NOAA will be requested when the pull request is marked ready for review DavidHuber-NOAA is a code owner

@CatherineThomas-NOAA CatherineThomas-NOAA Awaiting requested review from CatherineThomas-NOAA CatherineThomas-NOAA will be requested when the pull request is marked ready for review CatherineThomas-NOAA is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Marine DA script hardening
4 participants
@DavidNew-NOAA @aerorahul @guillaumevernieres @AndrewEichmann-NOAA