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

Update saber submodule to be inline with JCSDA/jedi-bundle #166

Merged
merged 15 commits into from
Sep 17, 2024

Conversation

SamuelDegelia-NOAA
Copy link
Contributor

@SamuelDegelia-NOAA SamuelDegelia-NOAA commented Sep 12, 2024

PR #121 updated the RDASApp submodules to be inline with JCSDA/jedi-bundle. All submodules were updated except for Saber due to a geometry issue that has now been resolved and merged. This PR thus updates the Saber submodule to its most recent develop commit. This resolves #163.

Test reference files have also been updated, and a new set of bump localization files are generated and staged on Hera and Jet under conus12km-401km11levels_20240912. Please note that before this PR is merged, I plan to overwrite the existing bumploc files and test reference data (they are not overwritten yet to give developers time for testing). So please do not merge this PR until we update the respective paths.

Note: In a future PR I would like to move the test reference data into the repo itself instead of being included in the staged data. That will make it easier for others to update it.

Ctest output:

(eva) [Samuel.Degelia@hfe01 rrfs-test]$ ctest
Test project /scratch1/NCEPDEV/da/Samuel.Degelia/RDASApp_dev/RDASApp/build/rrfs-test
    Start 1: rrfs_fv3jedi_hyb_2022052619
1/6 Test #1: rrfs_fv3jedi_hyb_2022052619 .........   Passed  137.10 sec
    Start 2: rrfs_fv3jedi_letkf_2022052619
2/6 Test #2: rrfs_fv3jedi_letkf_2022052619 .......   Passed   20.11 sec
    Start 3: rrfs_mpasjedi_2024052700_Ens3Dvar
3/6 Test #3: rrfs_mpasjedi_2024052700_Ens3Dvar ...   Passed   91.97 sec
    Start 4: rrfs_mpasjedi_2024052700_letkf
4/6 Test #4: rrfs_mpasjedi_2024052700_letkf ......   Passed   48.15 sec
    Start 5: rrfs_mpasjedi_2024052700_getkf
5/6 Test #5: rrfs_mpasjedi_2024052700_getkf ......   Passed   45.32 sec
    Start 6: rrfs_mpasjedi_2024052700_bumploc
6/6 Test #6: rrfs_mpasjedi_2024052700_bumploc ....   Passed  116.93 sec

100% tests passed, 0 tests failed out of 6

Label Time Summary:
mpi            = 459.59 sec*proc (6 tests)
rdas-bundle    = 459.59 sec*proc (6 tests)
script         = 459.59 sec*proc (6 tests)

Total Test time (real) = 460.11 sec

@ShunLiu-NOAA
Copy link

Thank you @SamuelDegelia-NOAA. It is a good idea to move test reference data into the repo.

@guoqing-noaa
Copy link
Collaborator

@SamuelDegelia-NOAA I am out of office this morning. I will review when I get back. One quick comment: please don't overwrite any fix files under RDAS_DATA. We need to track the changes and allow backward compatibility. Thanks!

@SamuelDegelia-NOAA
Copy link
Contributor Author

@guoqing-noaa None of the fix files are overwritten at the moment. New files/directories are created for testing. We can overwrite them only if this PR is approved.

TingLei-NOAA
TingLei-NOAA previously approved these changes Sep 12, 2024
@guoqing-noaa
Copy link
Collaborator

@guoqing-noaa None of the fix files are overwritten at the moment. New files/directories are created for testing. We can overwrite them only if this PR is approved.

@SamuelDegelia-NOAA we will never overwrite any fix file under RDAS_DATA. I will get back to you on this separately in the fix file management email communication this afternoon. Thanks!

@SamuelDegelia-NOAA
Copy link
Contributor Author

@SamuelDegelia-NOAA we will never overwrite any fix file under RDAS_DATA. I will get back to you on this separately in the fix file management email communication this afternoon. Thanks!

If that is the case, then I should update this PR with some more descriptive names for the new bumploc directories and reference files.

fix/bumploc/conus12km-401km11levels_updatesaber Outdated Show resolved Hide resolved
rrfs-test/scripts/link_mpasjedi_expr.sh Outdated Show resolved Hide resolved
rrfs-test/testinput/rrfs_mpasjedi_2024052700_Ens3Dvar.yaml Outdated Show resolved Hide resolved
rrfs-test/testinput/rrfs_mpasjedi_2024052700_Ens3Dvar.yaml Outdated Show resolved Hide resolved
@SamuelDegelia-NOAA SamuelDegelia-NOAA marked this pull request as draft September 12, 2024 20:26
@SamuelDegelia-NOAA SamuelDegelia-NOAA marked this pull request as ready for review September 13, 2024 03:32
@SamuelDegelia-NOAA
Copy link
Contributor Author

SamuelDegelia-NOAA commented Sep 13, 2024

All ctests work for a fresh clone/build after linking to the new paths for the bumploc and reference files. Please go ahead and re-review @TingLei-NOAA, @guoqing-noaa.

@guoqing-noaa
Copy link
Collaborator

@SamuelDegelia-NOAA

Two tests failed on Hercules and Jet:

The following tests FAILED:
          3 - rrfs_mpasjedi_2024052700_Ens3Dvar (Failed)
          6 - rrfs_mpasjedi_2024052700_bumploc (Failed)

and one test failed on Hera:

          6 - rrfs_mpasjedi_2024052700_bumploc (Failed)                            

Could you regenerate the 'ref" files for the above two cases?
Thanks!

@SamuelDegelia-NOAA
Copy link
Contributor Author

Oops, I didn't actually update Saber for my tests with the fresh clone. I needed to run git submodule update --init --recursive after switching to the branch because just changing branches doesn't update the submodules. So I did not catch the accidental overwriting of the ref files. I regenerated the ref files and will update RDAS_DATA/fix/expr_data/mpas_2024052700/ref_20240912 with them soon.

However, I am not sure why the rrfs_mpasjedi_2024052700_Ens3Dvar test is failing on Hercules/Jet and not Hera. Currently the rrfs-mpasjedi-ens3dvar.ref files are the same on each machine. I will do a fresh build on Jet to investigate further.

@SamuelDegelia-NOAA
Copy link
Contributor Author

SamuelDegelia-NOAA commented Sep 16, 2024

The issue with the staged data has been resolved and all ctests are passing for fresh clones on Hera and Jet. @guoqing-noaa will sync the data onto to Orion.

@guoqing-noaa
Copy link
Collaborator

@SamuelDegelia-NOAA Did we plot the single ob analysis increments using this new set of bumploc files? If yes, could you share it here? If not, we better make a plot on that. Thanks!

@SamuelDegelia-NOAA
Copy link
Contributor Author

Analysis increments using the updated bumploc files are posted here: #121 (comment)

@guoqing-noaa
Copy link
Collaborator

Analysis increments using the updated bumploc files are posted here: #121 (comment)

Thanks for reminding me of these plots!

@ShunLiu-NOAA
Copy link

@TingLei-NOAA please review this PR again before we merge it into develop.

@ShunLiu-NOAA ShunLiu-NOAA merged commit bf67744 into NOAA-EMC:develop Sep 17, 2024
1 check passed
@SamuelDegelia-NOAA SamuelDegelia-NOAA deleted the feature/update_saber branch September 17, 2024 15:34
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.

Update saber submodule
4 participants