Skip to content
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

Remove the eomg job #3331

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

DavidGrumm-NOAA
Copy link
Contributor

@DavidGrumm-NOAA DavidGrumm-NOAA commented Feb 14, 2025

Description

The eomg job is no longer supported for the GSI and the GDASApp has its own jobs to handle observational diagnostics (i.e. when lobsdiag=".true."): atmensanlobs and atmensanlsol. Thus, the eomg job will be removed from the global workflow without affecting the new atmensanlobs and atmensanlsol jobs.

This PR resolves issue #2525.

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/NO (If YES, please add a link to any PRs that are pending.)
    • [NO] EMC verif-global
    • [NO] GDAS
    • [NO] GFS-utils
    • [NO] GSI
    • [NO] GSI-monitor
    • [NO] GSI-utils
    • [NO] UFS-utils
    • [NO] UFS-weather-model
    • [NO] wxflow

How has this been tested?

Checklist

  • Any dependent changes have been merged and published
  • [X ] 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

@DavidGrumm-NOAA
Copy link
Contributor Author

@WalterKolczynski-NOAA @DavidHuber-NOAA @aerorahul : this PR is ready for review. There are some differences between the baseline output and the updated code output, which is unexpected. @DavidHuber-NOAA has suggested we go ahead with this PR and then rerun the test once approvals are made.

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.

In addition to the comments, there are some additional places where eomg needs to be removed. Specifically:

parm/config/gfs/config.resources*
parm/config/gfs/config.eobs (this config was used by eomg and there are a couple of eomg-specific variables)
parm/archive/enkf.yaml.j2

Also, there is still a mention of eomg in gfs_tasks.py (under the eupd method).

configs += ['ecen', 'esfc', 'efcs', 'echgres', 'epos', 'earc_vrfy']

if options['do_archtar']:
configs += ['earc_tars']
configs += ['ecen', 'esfc', 'efcs', 'echgres', 'epos', 'earc']
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be reverted.

Suggested change
configs += ['ecen', 'esfc', 'efcs', 'echgres', 'epos', 'earc_vrfy']
if options['do_archtar']:
configs += ['earc_tars']
configs += ['ecen', 'esfc', 'efcs', 'echgres', 'epos', 'earc']
configs += ['ecen', 'esfc', 'efcs', 'echgres', 'epos', 'earc_vrfy']
if options['do_archtar']:
configs += ['earc_tars']

Comment on lines +317 to +318
if options['lobsdiag_forenkf']:
task_names[run] += ['ediag']
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should just assume we are always running ediag and ignore the lobsdiag_forenkf flag when running GSI-based analyses (i.e. when do_jediatmens = False).

@DavidHuber-NOAA
Copy link
Contributor

Also, there are some checks for lobsdiag_forenkf in the following files that should be checked. If these checks are for JEDI-based analyses, then they're fine, otherwise the code should be simplified to remove the contents inside of the lobsdiag_forenkf logical blocks. Just a reminder, eomg used to be triggered when lobsdiag_forenkf = .false.

parm/archive/enkf_restarta_grp.yaml.j2
parm/archive/enkf_grp.yaml.j2
parm/config/gfs/config.anal
scripts/exgdas_enkf_update.sh
scripts/exglobal_atmos_analysis.sh

@DavidHuber-NOAA
Copy link
Contributor

@DavidGrumm-NOAA Per our discussion, please just remove eomg and lobsdiag references from GSI-specific files and not those files that are shared by both the GSI and GDASApp. Specifically, please target these files:

parm/config/gfs/config.anal
scripts/exgdas_enkf_update.sh
scripts/exglobal_atmos_analysis.sh
parm/config/gfs/config.resources
parm/config/gfs/config.resources.HERCULES
parm/config/gfs/config.eobs (this config was used by eomg and there are a couple of eomg-specific variables)

@DavidHuber-NOAA DavidHuber-NOAA changed the title Remove the eomg job issue Remove the eomg job Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants