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

Marine B-matrix refactor companion PR #1346

Merged
merged 28 commits into from
Nov 6, 2024
Merged

Conversation

DavidNew-NOAA
Copy link
Collaborator

This PR is a companion to Global Workflow PR #2992 and JCB-GDAS PR #36.

It makes a few necessary changes to the marine JCB base YAML required to have JCB working in the B-matrix task in the Global Workflow.

danholdaway
danholdaway previously approved these changes Oct 24, 2024
danholdaway added a commit to NOAA-EMC/jcb-gdas that referenced this pull request Nov 1, 2024
This PR is a companion to Global Workflow PR
[#2992](NOAA-EMC/global-workflow#2992) and
GDASApp PR [#1346](NOAA-EMC/GDASApp#1346).

It adds JCB templates for the input YAMLS of JEDI applications used in
the marine B-matrix task in the Global Workflow.

---------

Co-authored-by: Dan Holdaway <27729500+danholdaway@users.noreply.github.com>
Co-authored-by: Guillaume Vernieres <guillaume.vernieres@noaa.gov>
@DavidNew-NOAA
Copy link
Collaborator Author

@guillaumevernieres @AndrewEichmann-NOAA @danholdaway @CoryMartin-NOAA Can I get a final review on this PR, so we can merge and hopefully move forward on GW PR #2992 next week?

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.

Looks good to me, merge once @guillaumevernieres is happy

@DavidNew-NOAA
Copy link
Collaborator Author

This PR will break marine testing until GW PR #2992 is merged, so I might as well wait until I meet with Rahul and Walter next week to hash out the issues with that PR before I merge this one. The companion jcb-gdas PR that just got merged shouldn't break anything though, so that's good.

@guillaumevernieres
Copy link
Contributor

This PR will break marine testing until GW PR #2992 is merged, so I might as well wait until I meet with Rahul and Walter next week to hash out the issues with that PR before I merge this one. The companion jcb-gdas PR that just got merged shouldn't break anything though, so that's good.

develop is broken anyways, the default state ... But yes, lets hold on!

Copy link
Collaborator

@AndrewEichmann-NOAA AndrewEichmann-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

@DavidNew-NOAA
Copy link
Collaborator Author

I'm going to go ahead and merge this PR since GW PR #2992 should be ready for review soon after discussing with the GW team yesterday. Let me know if anyone objects.

@guillaumevernieres
Copy link
Contributor

I'm going to go ahead and merge this PR since GW PR #2992 should be ready for review soon after discussing with the GW team yesterday. Let me know if anyone objects.

Go for it. We'll have bugfixes coming in after yours (#1359), but what you have in this PR should still be OK with the low-res test cases.

@DavidNew-NOAA DavidNew-NOAA merged commit e9607fc into develop Nov 6, 2024
5 checks passed
@DavidNew-NOAA DavidNew-NOAA deleted the feature/marine-refactor branch November 6, 2024 13:53
WalterKolczynski-NOAA pushed a commit to NOAA-EMC/global-workflow that referenced this pull request Nov 19, 2024
…efactoring (#2992)

This PR is a companion to GDASApp PR
NOAA-EMC/GDASApp#1312 (merged) and NOAA-EMC/jcb-gdas#31
(merged).

This PR does three things:

1. It changes the observation and bias staging for the atmospheric
analysis tasks to use JCB templates instead of reading the full JEDI
input configuration dictionary in order to construct a list of files to
stage. This is cleaner and places fewer constraints on how to initialize
the analysis.
2. The ```Jedi``` constructor now takes as input a dictionary that is
essentially subset of the ```task_config``` dictionary. This makes the
code clearer and less opaque and makes debugging easier. Each dictionary
is constructed from a YAML file with configuration parameters for each
JEDI application that is run.
3. All JEDI applications and their input YAMLs are now initialized in
the initialize job of the ```AtmAnalysis``` and ```AtmEnsAnalysis```.
Before, in the ```atmensanl*``` jobs for example, the LETKF solver was
initialized in the ```atmensanlinit```cjob, but the LETKF solver and FV3
increment converter were both initialized and executed in the
```atmensanlobs``` and ```atmensanlfv3inc``` jobs respectively. This
makes more sense in terms of resource allocation.

Addendum:

I'm now rolling in the refactoring of the marine B-matrix task into this
PR. That makes it also a companion of NOAA-EMC/GDASApp#1346 and
NOAA-EMC/jcb-gdas#36.

These new changes introduce the ```Jedi``` class and JCB into the marine
B-matrix job.

Partially resolvesNOAA-EMC/GDASApp#1296

---------

Co-authored-by: RussTreadon-NOAA <Russ.Treadon@noaa.gov>
Co-authored-by: Rahul Mahajan <aerorahul@users.noreply.github.com>
WalterKolczynski-NOAA pushed a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Nov 22, 2024
…efactoring (NOAA-EMC#2992)

This PR is a companion to GDASApp PR
NOAA-EMC/GDASApp#1312 (merged) and NOAA-EMC/jcb-gdas#31
(merged).

This PR does three things:

1. It changes the observation and bias staging for the atmospheric
analysis tasks to use JCB templates instead of reading the full JEDI
input configuration dictionary in order to construct a list of files to
stage. This is cleaner and places fewer constraints on how to initialize
the analysis.
2. The ```Jedi``` constructor now takes as input a dictionary that is
essentially subset of the ```task_config``` dictionary. This makes the
code clearer and less opaque and makes debugging easier. Each dictionary
is constructed from a YAML file with configuration parameters for each
JEDI application that is run.
3. All JEDI applications and their input YAMLs are now initialized in
the initialize job of the ```AtmAnalysis``` and ```AtmEnsAnalysis```.
Before, in the ```atmensanl*``` jobs for example, the LETKF solver was
initialized in the ```atmensanlinit```cjob, but the LETKF solver and FV3
increment converter were both initialized and executed in the
```atmensanlobs``` and ```atmensanlfv3inc``` jobs respectively. This
makes more sense in terms of resource allocation.

Addendum:

I'm now rolling in the refactoring of the marine B-matrix task into this
PR. That makes it also a companion of NOAA-EMC/GDASApp#1346 and
NOAA-EMC/jcb-gdas#36.

These new changes introduce the ```Jedi``` class and JCB into the marine
B-matrix job.

Partially resolvesNOAA-EMC/GDASApp#1296

---------

Co-authored-by: RussTreadon-NOAA <Russ.Treadon@noaa.gov>
Co-authored-by: Rahul Mahajan <aerorahul@users.noreply.github.com>
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.

5 participants