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

Changing duplicate product ids from local use to WMO #827

Merged
merged 8 commits into from
Dec 13, 2023

Conversation

AndrewBenjamin-NOAA
Copy link
Contributor

Removed <table_info>NCEP</table_info> for several parameters in fv3lam_rrfs.xml and postxconfig-NT-fv3lam_rrfs.txt.

This allows for the WMO parameter ids to be used over the duplicate NCEP local use ids, exclusively in the RRFS. Other operational models are not impacted.

resolves #826

@WenMeng-NOAA
Copy link
Collaborator

@AndrewBenjamin-NOAA Can you sync your branch with latest develop branch? Do you have the UPP standalone test for RRFS available?

@AndrewBenjamin-NOAA
Copy link
Contributor Author

@WenMeng-NOAA I have synched with the latest dev branch. What is the best way to run a RRFS standalone test on WCOSS2?

@WenMeng-NOAA
Copy link
Collaborator

@AndrewBenjamin-NOAA You may pick up the driver script of the RRFS case at /u/wen.meng/noscrub/ncep_post/develop/UPP/submit_run_post_rrfs_wcoss2.sh. You run your test with your branch, verify the expected changes, and provide me your test results on WCOSS2.

@AndrewBenjamin-NOAA
Copy link
Contributor Author

@WenMeng-NOAA thanks for providing that path (again). I ran a standalone test and confirmed that the proper wmo ids were changed. You may see the results in /lfs/h2/emc/ptmp/andrew.benjamin/post_rrfs_2023062800.165793.

test files degrib output:
degrib.test.prslev10
degrib.test.natlev10

current RRFS_A parallel output:
degrib.parallel.prslev
degrib.parallel.natlev

@WenMeng-NOAA
Copy link
Collaborator

@MatthewPyle-NOAA and @BenjaminBlake-NOAA Please let me know if you have any concerns/suggestion on these RRFS product changes. Thanks!

@AndrewBenjamin-NOAA
Copy link
Contributor Author

AndrewBenjamin-NOAA commented Dec 6, 2023

@WenMeng-NOAA yes the LFTX needs to be changed and I actually found another field (4LFTX 180-0mb) that also needs to be changed. I'll will add a commit to include that field as well.

The emphasis is on parameters that have WMO headers and go out over the SBN, but eventually all grib parameters that use local use id's that also have official WMO ids must change to use the WMO id. We need these changes so that WMO headers can be properly created and ingested for the RRFS. This is what the SBN folks want, and these are the first steps in a very long transition, starting with the RRFS v1.0 implementation. New model, consistent ids.

@BenjaminBlake-NOAA
Copy link
Collaborator

@WenMeng-NOAA Thanks for reaching out! @AndrewBenjamin-NOAA one question I have is related to the contents of your degrib.test files in your post_rrfs_2023062800.165793 directory on Cactus. I noticed many parameters are changed to UNKNOWN when comparing degrib.test.prslev10 to degrib.parallel.prslev, such as grib message 3 (REFC). Why is that the case? When I run wgrib2 -V on the prslev10.grib2 file, the parameters are listed as expected, so this may not be an issue but I wanted to check.

@AndrewBenjamin-NOAA
Copy link
Contributor Author

I noticed many parameters are changed to UNKNOWN when comparing degrib.test.prslev10 to degrib.parallel.prslev, such as grib message 3 (REFC). Why is that the case? When I run wgrib2 -V on the prslev10.grib2 file, the parameters are listed as expected, so this may not be an issue but I wanted to check.

@BenjaminBlake-NOAA this is specifically related to the degrib2 tool. I suspect it needs to be updated, and I have mentioned this to the grib_utils team. But since everything still works in wgrib2, it will not impact regridding, parsing out data, etc.

The UNKNOWN in degrib2 isn't really an issue because the ids are now correct. If someone wanted to they could look up the id in the NCEP grib2 documentation. The mnemonic is something only we do at NCEP, it is WMO mandated, what is important is all of the other information that degrib2 shows, which is now consistent with the WMO.

@WenMeng-NOAA
Copy link
Collaborator

I noticed many parameters are changed to UNKNOWN when comparing degrib.test.prslev10 to degrib.parallel.prslev, such as grib message 3 (REFC). Why is that the case? When I run wgrib2 -V on the prslev10.grib2 file, the parameters are listed as expected, so this may not be an issue but I wanted to check.

@BenjaminBlake-NOAA this is specifically related to the degrib2 tool. I suspect it needs to be updated, and I have mentioned this to the grib_utils team. But since everything still works in wgrib2, it will not impact regridding, parsing out data, etc.

The UNKNOWN in degrib2 isn't really an issue because the ids are now correct. If someone wanted to they could look up the id in the NCEP grib2 documentation. The mnemonic is something only we do at NCEP, it is WMO mandated, what is important is all of the other information that degrib2 shows, which is now consistent with the WMO.

@AndrewBenjamin-NOAA You might check the degrib2 version consistency for generating degrib.parallel.prslev and degrib.test.prslev10.

@AndrewBenjamin-NOAA
Copy link
Contributor Author

@WenMeng-NOAA they are the same version. I ran a wgrib2 -match to pull the fields I knew I wanted to investigate. The parallel data is constantly updating with new fields so it grabbed more grib2 records than the UPP test.

The important thing is that it confirmed the changes I made had the desired effect to those specific fields, not the total number of fields in the output, because only specific fields were changed.

@BenjaminBlake-NOAA
Copy link
Collaborator

@AndrewBenjamin-NOAA Thanks for confirming! The changes look good to me.

@WenMeng-NOAA
Copy link
Collaborator

@AndrewBenjamin-NOAA Would you like to add the change of 4LFTX in this PR?

@WenMeng-NOAA
Copy link
Collaborator

@AndrewBenjamin-NOAA Please sync your branch.

@AndrewBenjamin-NOAA
Copy link
Contributor Author

@WenMeng-NOAA I just did, there was another commit to develop so I wanted to bring those changes in first

@WenMeng-NOAA WenMeng-NOAA added RRFS Ready for Review This PR is ready for code review. Baseline Change The baselines of the UPP regression tests are changed. labels Dec 8, 2023
@WenMeng-NOAA
Copy link
Collaborator

@AndrewBenjamin-NOAA It seems to me you missed the change in the field "REFD:263 K level".

@AndrewBenjamin-NOAA
Copy link
Contributor Author

@WenMeng-NOAA My initial priority was to change only the products that required WMO headers (hence going out over the SBN).

I can see how having 2 grib2 ids would be problematic, so it may make sense to change them all at once. Do you recommend doing that here for this PR, or in a separate one?

@WenMeng-NOAA
Copy link
Collaborator

@WenMeng-NOAA My initial priority was to change only the products that required WMO headers (hence going out over the SBN).

I can see how having 2 grib2 ids would be problematic, so it may make sense to change them all at once. Do you recommend doing that here for this PR, or in a separate one?

@AndrewBenjamin-NOAA Thanks for clarifying. Since this is minor change, I would recommend you include in this PR.

@AndrewBenjamin-NOAA
Copy link
Contributor Author

@WenMeng-NOAA, I will make the change to "REFD: 263K Level" for this PR.

There are other products that are not going over the SBN that will also need their IDs changed. I will save them for a separate PR because it is a much bigger list and I must make sure I have them all.

@WenMeng-NOAA
Copy link
Collaborator

@WenMeng-NOAA, I will make the change to "REFD: 263K Level" for this PR.

There are other products that are not going over the SBN that will also need their IDs changed. I will save them for a separate PR because it is a much bigger list and I must make sure I have them all.

@AndrewBenjamin-NOAA Your plan sounds good to me. We will wrap up your PR for final testing.

@WenMeng-NOAA
Copy link
Collaborator

The UPP RTs were completed on Hera. There will be baseline recreation for fv3r and 3drtma.

@WenMeng-NOAA
Copy link
Collaborator

@FernandoAndrade-NOAA You may start the UPP RTs on R&D platforms at EPIC side and refer to my testing results on Hera at /scratch1/NCEPDEV/stmp2/Wen.Meng as;

[Wen.Meng@hfe01 ~/stmp2]$ ls -ltr */*diff
-rw-r--r-- 1 Wen.Meng stmp  1831 Dec 13 03:24 fv3r_2023062800/PRSLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp 14552 Dec 13 03:25 rtma_2023040400_pe_test/NATLEV00.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp 14401 Dec 13 03:26 fv3r_2023062800/NATLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp  2352 Dec 13 03:26 rtma_2023040400_pe_test/PRSLEV00.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp  1831 Dec 13 03:27 fv3r_2023062800_pe_test/PRSLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp    75 Dec 13 03:27 rap_2020072316_pe_test/WRFPRS.GrbF16.diff
-rw-r--r-- 1 Wen.Meng stmp 14552 Dec 13 03:28 rtma_2023040400/NATLEV00.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp 14401 Dec 13 03:29 fv3r_2023062800_pe_test/NATLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp  2352 Dec 13 03:30 rtma_2023040400/PRSLEV00.tm00.diff

@FernandoAndrade-NOAA
Copy link
Collaborator

@FernandoAndrade-NOAA You may start the UPP RTs on R&D platforms at EPIC side and refer to my testing results on Hera at /scratch1/NCEPDEV/stmp2/Wen.Meng as;

[Wen.Meng@hfe01 ~/stmp2]$ ls -ltr */*diff
-rw-r--r-- 1 Wen.Meng stmp  1831 Dec 13 03:24 fv3r_2023062800/PRSLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp 14552 Dec 13 03:25 rtma_2023040400_pe_test/NATLEV00.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp 14401 Dec 13 03:26 fv3r_2023062800/NATLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp  2352 Dec 13 03:26 rtma_2023040400_pe_test/PRSLEV00.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp  1831 Dec 13 03:27 fv3r_2023062800_pe_test/PRSLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp    75 Dec 13 03:27 rap_2020072316_pe_test/WRFPRS.GrbF16.diff
-rw-r--r-- 1 Wen.Meng stmp 14552 Dec 13 03:28 rtma_2023040400/NATLEV00.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp 14401 Dec 13 03:29 fv3r_2023062800_pe_test/NATLEV10.tm00.diff
-rw-r--r-- 1 Wen.Meng stmp  2352 Dec 13 03:30 rtma_2023040400/PRSLEV00.tm00.diff

The changes in my Orion tests seem to match yours on Hera, I will update baselines on both machines once merged in. Thanks for testing on Hera!

@WenMeng-NOAA
Copy link
Collaborator

This PR is ready for merging.

@WenMeng-NOAA WenMeng-NOAA merged commit d7ab9b8 into NOAA-EMC:develop Dec 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Change The baselines of the UPP regression tests are changed. Ready for Review This PR is ready for code review. RRFS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing duplicate product ids from local use to WMO in the RRFS
4 participants