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

Oct 2024 Variable Change Sprint #457

Merged
merged 19 commits into from
Dec 4, 2024
Merged

Conversation

rtodling
Copy link
Contributor

@rtodling rtodling commented Nov 19, 2024

Description

All this is using JEDI from 19Nov2024. So this is following JEDI build update to November 19th

Working to address changes from October 2024 Variable Change Sprint.

  • Geoval names were updated and relevant R2D2 files and folders were updated to handle this change. Now new folders are named v3 and set as the default
  • Erased some of the overwrites in Tier 1 tests to prevent confusion
  • fields_metadata.yaml changed for SOCA
  • Sea-ice variable names changed.

These now work for all of the atmospheric obs systems:

However, in the SSMI case, I am seeing the Jo got down to negative numbers ... this means something is odd somewhere - bias correction, or something. I am not sure whether this was the case in the original code (before all these changes) ... I am currently checking on that.

SOCA notes:

There are major changes in terms of what internal variable names SOCA would be using and hence the configuration yamls needed to be updated. In terms of MOM6 outputs or obs files, there are no changes. See more details in JCSDA-internal/soca#1082. For instance instead of tocn we set sea_water_potential_temperature.

It also conforms with JCSDA-internal/soca#1085, BkgErrGODAS is moved out to the generic block. Most of the default balance values were what we have been using but there is an additional OceanSmoother that we should be careful about.

@rtodling
Copy link
Contributor Author

General comment about what will be required in terms of getting the UFO test working -- we'll need to go over the variable name changes seen in the yaml; look for the geovals that changed names, and rename those variables in the geoval files of the corresponding instruments. This is a relatively simple task ... I might work on that later today.

…... I wonder if this was the case in the original code, but had gone unnoticed - I am checking
@rtodling
Copy link
Contributor Author

So I am confirming that the problem I see w/ SSMI/S is already in develop:

Quadratic cost function: JoJc( 0) = 2904.536994156287
Quadratic cost function: JoJc( 1) = 1761.685897988107
Quadratic cost function: JoJc( 2) = 41.58146053894939
Quadratic cost function: JoJc( 3) = -2188.836274347395
Quadratic cost function: JoJc( 4) = -4574.266068595534
Quadratic cost function: JoJc( 5) = -7173.171878308596
Quadratic cost function: JoJc( 6) = -8398.747569179868
Quadratic cost function: JoJc( 7) = -9260.910107256032
Quadratic cost function: JoJc( 8) = -9691.212019756096
Quadratic cost function: JoJc( 9) = -9915.784237898803
Quadratic cost function: JoJc(10) = -10072.29704553444

So the changes introduced in this PR are not related to this issue. I will open an issue regarding this.

@rtodling
Copy link
Contributor Author

At this stage, I say that all work to merge w/ Sprint (19Nov2024 version of JEDI) has been done for VAR.

Work needs to be done for:

SOCA and HOFX/UFO

@Dooruk
Copy link
Collaborator

Dooruk commented Nov 21, 2024

Can you share the November build you are using so I can also test with it?

@rtodling
Copy link
Contributor Author

I have now a script that converts the variables in the geovals files according to the Sprint changes. I have triggered the UFO testing for all obs types ... except for aircraft and gps which seems to give me an unrelated error - all seems to work. I think I might be able to figure out the gps one ... the aircraft relates to a bias correction issue - not clear what's going on.

The only thing I have not tested - cause I don't know how - is SOCA. But for atmosphere I now believe to have all needed changes. Somebody w/ super-powers will need to run the script I created over the geovals files stored in the original place.

@rtodling
Copy link
Contributor Author

rtodling commented Nov 28, 2024

(Doruk) This is done now:

The following following should be done to allow the UFO tests to working for versions of JEDI pass the Oct 2024 Code Sprint:

cp -r /gpfsm/dnb33/rtodling/R2D2DataStore/Shared/ncdiag/ob/x0048v3-geovals /discover/nobackup/projects/gmao/advda/R2D2DataStore/Shared/ncdiag/ob/

@Dooruk Dooruk marked this pull request as ready for review December 4, 2024 14:31
@Dooruk Dooruk requested review from metdyn and gmao-wgu December 4, 2024 14:34
@Dooruk Dooruk requested a review from gmao-yzhu December 4, 2024 14:34
@rtodling
Copy link
Contributor Author

rtodling commented Dec 4, 2024

@Dooruk why do we need all these requests for review?

@Dooruk
Copy link
Collaborator

Dooruk commented Dec 4, 2024

@Dooruk why do we need all these requests for review?

It's more of a heads up for everybody but we can go ahead and merge it

@Dooruk Dooruk self-requested a review December 4, 2024 18:08
Copy link
Collaborator

@Dooruk Dooruk left a comment

Choose a reason for hiding this comment

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

3dvar still fails on the runners but not during local tests, but I don't want to hold this back. I asked SI-team and @mranst 's help on this.

@Dooruk Dooruk merged commit 28e88f1 into develop Dec 4, 2024
2 checks passed
@Dooruk Dooruk deleted the feature/rtodling/sprint_atm_sondes branch December 4, 2024 18:11
@gmao-msienkie
Copy link
Contributor

I ran swell-convert_ncdiags-suite and the ncdiag file for aircraft that was generated still has the 'windUpward' name in the metadata and not the new 'instantaneousAltitudeRate'. I guess that converter needs an update as well.

@Dooruk
Copy link
Collaborator

Dooruk commented Dec 6, 2024

I ran swell-convert_ncdiags-suite and the ncdiag file for aircraft that was generated still has the 'windUpward' name in the metadata and not the new 'instantaneousAltitudeRate'. I guess that converter needs an update as well.

Sorry, SLES12 build wasn't updated to 11/19 but it is now 👍

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.

3 participants