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

Create JEDI class #2805

Merged

Conversation

DavidNew-NOAA
Copy link
Contributor

Description

This PR creates a PyGFS class called JEDI, which is to be instantiated everytime a JEDI application is run. The AtmAnalysis and AtmEnsAnalysis classes are no longer children of the Analysis class, but rather direct children of the Task class. They each have a JEDI object as an attribute, which is used to run either the variational/ensemble DA JEDI applications or the FV3 increment converter JEDI application, depending on which job they are created for (e.g. atmanlvar vs. atmanlfv3inc). The intention is that a later PR will apply this framework to all analysis task, and the PyGFS Analysis class will be removed.

Type of change

  • New feature (adds functionality)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? YES
  • Does this change require a documentation update? NO

How has this been tested?

This is just a draft as of right now.

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
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • I have made corresponding changes to the documentation if necessary

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-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 don't have any major comments here, but I'm worried that we may be missing something that would make this more generic. @DavidNew-NOAA do you think it is reasonable to try this for another app (snow, aero, soca) at this stage to see if the JEDI class is generic enough?

@DavidNew-NOAA
Copy link
Contributor Author

@CoryMartin-NOAA Sure, good idea. I haven't run any of those analyses in the past, but I'm willing to give them a try with the JEDI class.

@CoryMartin-NOAA
Copy link
Contributor

@DavidNew-NOAA there are CI test cases for all of the above (I think). I would suggest starting with snow DA as it is the simplest, then aero. Let me know if you need help setting up an experiment using the CI test case(s)

@DavidNew-NOAA
Copy link
Contributor Author

DavidNew-NOAA commented Aug 2, 2024

@guillaumevernieres Feel free to take a look at what I've done here. I'm still in draft mode.

Re: your comments on my previous (now closed) PR for context:

That PR was based on the idea of an inheritance family tree like so: Task->JEDI->AtmAnalysis/AtmEnsAnalysis/etc, but after some discussion with @aerorahul we are going to make AtmAnalysis/AtmEnsAnalysis/etc direct children of the Task class, and make the JEDI class an attribute of AtmAnalysis/AtmEnsAnalysis/etc. So every time you want to run a JEDI application in a task, you instantiate a JEDI object which contains all the information and methods necessary for initializing and running that application.

You mentioned that sometimes you want to run multiple JEDI applications in one set of tasks. It's a bit subtle, but in the case of AtmAnalysis, for example, that class only creates one JEDI object, but depending on whether I'm running the gdasatmanlvar job or the gdasatmanlfv3inc job, that JEDI object received a different JEDIEXE and JCB_BASE_YAML environment variable from config.atmanl+config.atmanlvar vs config.atmanlfv3inc, and thus they run a different JEDI executable with a different YAML depending on the job.

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.

Looks good @DavidNew-NOAA and it makes more sense if it's meant to be used to create data members in our tasks rather than inherit a rigid framework. That should replace part of a utility module we currently use for our b-matrix job.

I can see how to use your work to cleanup the marine-da/soca b-matrix task and run multiple jedi applications ... but we're so far behind on so many other things that it probably won't happen for a while!

@RussTreadon-NOAA
Copy link
Contributor

@DavidNew-NOAA , I am about to begin work on g-w issue #2415 in RussTreadon-NOAA:feature/ensda_obs.

Do you recommend that I wait for this JEDI class PR to be merged into develop before adding a JEDI eobs equivalent to g-w?

@DavidNew-NOAA
Copy link
Contributor Author

@RussTreadon-NOAA I still need Rahul's feedback, and he'a on leave, so I wouldn't wait.

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.

I only reviewed jedi.py as that sets the foundation for this PR. I am open to your (and others) thoughts on my comments.

ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/jedi.py Outdated Show resolved Hide resolved
@DavidNew-NOAA DavidNew-NOAA marked this pull request as ready for review August 22, 2024 13:37
@emcbot emcbot added CI-Orion-Building **Bot use only** CI testing is cloning/building on Orion CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS and removed CI-Hercules-Ready **CM use only** PR is ready for CI testing on Hercules CI-Orion-Ready **CM use only** PR is ready for CI testing on Orion CI-Wcoss2-Ready **CM use only** PR is ready for CI testing on WCOSS labels Sep 6, 2024
@emcbot
Copy link

emcbot commented Sep 6, 2024

CI Update on Wcoss2 at 09/06/24 03:12:11 PM
============================================
Cloning and Building global-workflow PR: 2805
with PID: 186518 on host: dlogin03

@emcbot
Copy link

emcbot commented Sep 6, 2024

Build FAILED on Orion in Build# 4 with error logs:

/work2/noaa/stmp/CI/ORION/2805/gfs/sorc/logs/build_upp.log

Follow link here to view the contents of the above file(s): (link)

@emcbot emcbot added CI-Orion-Failed **Bot use only** CI testing on Orion for this PR has failed and removed CI-Orion-Building **Bot use only** CI testing is cloning/building on Orion labels Sep 6, 2024
@WalterKolczynski-NOAA WalterKolczynski-NOAA removed the CI-Orion-Failed **Bot use only** CI testing on Orion for this PR has failed label Sep 6, 2024
@emcbot emcbot added CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress and removed CI-Hercules-Building **Bot use only** CI testing is cloning/building on Hercules labels Sep 6, 2024
@WalterKolczynski-NOAA
Copy link
Contributor

Ignore Orion. Workflow in general doesn't work there yet.

@emcbot emcbot added CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress and removed CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS labels Sep 6, 2024
@emcbot
Copy link

emcbot commented Sep 6, 2024

Automated global-workflow Testing Results:

Machine: Wcoss2
Start: Fri Sep  6 15:18:22 UTC 2024 on dlogin03
---------------------------------------------------
Build: Completed at 09/06/24 04:06:13 PM
Case setup: Completed for experiment C48_ATM_f445e28d
Case setup: Skipped for experiment C48mx500_3DVarAOWCDA_f445e28d
Case setup: Skipped for experiment C48_S2SWA_gefs_f445e28d
Case setup: Completed for experiment C48_S2SW_f445e28d
Case setup: Completed for experiment C96_atm3DVar_extended_f445e28d
Case setup: Skipped for experiment C96_atm3DVar_f445e28d
Case setup: Completed for experiment C96C48_hybatmaerosnowDA_f445e28d
Case setup: Completed for experiment C96C48_hybatmDA_f445e28d
Case setup: Completed for experiment C96C48_ufs_hybatmDA_f445e28d

@aerorahul
Copy link
Contributor

Ignore Orion. Workflow in general doesn't work there yet.

The workflow does with the exception of cycling because the UPP submodule. It is being addressed in #2877

@emcbot emcbot added CI-Hercules-Passed **Bot use only** CI testing on Hercules for this PR has completed successfully and removed CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress labels Sep 6, 2024
@emcbot
Copy link

emcbot commented Sep 6, 2024

CI Passed on Hercules in Build# 3
Built and ran in directory /work2/noaa/stmp/CI/HERCULES/2805


Experiment C48_ATM_f445e28d Completed 1 Cycles: *SUCCESS* at Fri Sep  6 12:18:24 CDT 2024
Experiment C48_S2SW_f445e28d Completed 1 Cycles: *SUCCESS* at Fri Sep  6 13:18:59 CDT 2024
Experiment C96_atm3DVar_f445e28d Completed 3 Cycles: *SUCCESS* at Fri Sep  6 13:25:15 CDT 2024
Experiment C96C48_hybatmDA_f445e28d Completed 3 Cycles: *SUCCESS* at Fri Sep  6 13:31:23 CDT 2024
Experiment C48_S2SWA_gefs_f445e28d Completed 1 Cycles: *SUCCESS* at Fri Sep  6 14:44:43 CDT 2024

@emcbot emcbot added CI-Wcoss2-Passed **Bot use only** CI testing on WCOSS for this PR has completed successfully and removed CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress labels Sep 7, 2024
@emcbot
Copy link

emcbot commented Sep 7, 2024

All CI Test Cases Passed on Wcoss2:

Experiment C48_ATM_f445e28d *** SUCCESS *** at 09/06/24 05:36:07 PM
Experiment C48_S2SW_f445e28d *** SUCCESS *** at 09/06/24 05:45:14 PM
Experiment C96C48_hybatmDA_f445e28d *** SUCCESS *** at 09/06/24 07:06:23 PM
Experiment C96C48_hybatmaerosnowDA_f445e28d *** SUCCESS *** at 09/06/24 07:36:21 PM
Experiment C96C48_ufs_hybatmDA_f445e28d *** SUCCESS *** at 09/06/24 08:57:15 PM
Experiment C96_atm3DVar_extended_f445e28d *** SUCCESS *** at 09/07/24 06:06:40 AM

@WalterKolczynski-NOAA WalterKolczynski-NOAA merged commit 49f697a into NOAA-EMC:develop Sep 7, 2024
10 of 11 checks passed
@RussTreadon-NOAA
Copy link
Contributor

Thank you @WalterKolczynski-NOAA!

DavidHuber-NOAA added a commit to DavidHuber-NOAA/global-workflow that referenced this pull request Sep 9, 2024
* origin/develop:
  Create JEDI class (NOAA-EMC#2805)
  Restructure the bufr sounding job    (NOAA-EMC#2853)
  Add an archive task to GEFS system to archive files locally (NOAA-EMC#2816)
  Reenable Orion Cycling Support (NOAA-EMC#2877)
  Eliminate race conditions and remove DATAROOT last in cleanup (NOAA-EMC#2893)
  Update aerosol climatology to 2013-2024 mean (NOAA-EMC#2888)
  Add ability to run CI test C96_atm3DVar.yaml to Gaea-C5 (NOAA-EMC#2885)
  Support global-workflow GEFS C48 on Google Cloud (NOAA-EMC#2861)
  Add 3 and 9 hr increment files to IC staging (NOAA-EMC#2876)
  Add diffusion/diag B for aerosol DA and some other needed changes (NOAA-EMC#2738)
  Correct ocean `MOM.res_#` stage copy (NOAA-EMC#2868)
  Support coupling on AWS (NOAA-EMC#2859)
  Add JEDI ATM lgetkf observer and solver jobs (NOAA-EMC#2833)
  Fix gdas build on Gaea and add Gaea to available CI list (NOAA-EMC#2857)
  Support ATM forecast only on Google (NOAA-EMC#2832)
  Add GEFS C48 support on AWS (NOAA-EMC#2818)
  Update omega calculation (NOAA-EMC#2751)
  Add snow DA update and recentering for the EnKF forecasts (NOAA-EMC#2690)
  support ATM forecast only on Azure (NOAA-EMC#2827)
  Convert staging job to python and yaml (NOAA-EMC#2651)
  Fixed test on UNAVAILBLE in python Rocoto check (NOAA-EMC#2842)
@DavidNew-NOAA DavidNew-NOAA deleted the feature/jedi_class branch September 11, 2024 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-Hera-Passed **Bot use only** CI testing on Hera for this PR has completed successfully CI-Hercules-Passed **Bot use only** CI testing on Hercules for this PR has completed successfully CI-Wcoss2-Passed **Bot use only** CI testing on WCOSS for this PR has completed successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants