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

Additional GTG output "CIT" #839

Merged
merged 17 commits into from
Jan 25, 2024
Merged

Conversation

hsinmulin-NOAA
Copy link
Contributor

@hsinmulin-NOAA hsinmulin-NOAA commented Dec 11, 2023

Related changes for GTG new output "CIT" in the source code & model input files.
The new "CIT" (CITEDR in GRID2 Table)
("0 19 50 0 CITEDR" as in https://www.nco.ncep.noaa.gov/pmb/docs/grib2/grib2_doc/grib2_table4-2-0-19.shtml)

In additional to the added new variable, the consistency of UPP outputs with or without variables from GTG are considered.
The new files will be committed according to this update are

/sorc/ncep_post.fd
MISCLN.f
MDLFLD.f
gtg_interp.F90 : Stub code for GTG protection but to make UPP public to work
"CMakeLists.txt" to include new file "gtg_interp.F90" from GTG. #840

/parm
fv3lam_rrfs.xml
params_grib2_tbl_new.text
params_grib2_tbl_new
post_avblflds.xml
postxconfig-NT-fv3lam_rrfs.txt

New GTG_4.15 version link (protected)

add "CIT" as additional GTG output
additional "CIT" related GTG output changes for the UPP
additional GTG output variable "CIT"
@FernandoAndrade-NOAA
Copy link
Collaborator

Hi @hsinmulin-NOAA, please update the changelog entries at the top of MISCLN.f for your updates and create/link a git issue for this pr for tracking. Thank you

@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Dec 13, 2023 via email

@YaliMao-NOAA
Copy link
Contributor

@hsinmulin-NOAA Based on the latest GTG code delivery, may you please include the update of sorc/ncep_post.fd/CMakeLists.txt by adding gtg_interp.F90?

@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Dec 18, 2023 via email

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA Please sync your branch with the latest UPP develop and rename your folk from EMC_post to UPP. You may follow the instructions at #372 to rename folk.

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA Based on the latest GTG code delivery, may you please include the update of sorc/ncep_post.fd/CMakeLists.txt by adding gtg_interp.F90?

@YaliMao-NOAA and @hsinmulin-NOAA I do not see gtg_interp.F90 from the submodule post_gtg with commit @a982870.

Add "gtg_interp.F90" per updated GTG_4.15 version that pulls all the interpolation to this file.
@hsinmulin-NOAA
Copy link
Contributor Author

@hsinmulin-NOAA Based on the latest GTG code delivery, may you please include the update of sorc/ncep_post.fd/CMakeLists.txt by adding gtg_interp.F90?

@YaliMao-NOAA and @hsinmulin-NOAA I do not see gtg_interp.F90 from the submodule post_gtg with commit @a982870.

Extra line in "CMakeLists.txt" to include new file "gtg_interp.F90" from GTG was added.
Per issue #840

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA Based on the latest GTG code delivery, may you please include the update of sorc/ncep_post.fd/CMakeLists.txt by adding gtg_interp.F90?

@YaliMao-NOAA and @hsinmulin-NOAA I do not see gtg_interp.F90 from the submodule post_gtg with commit @a982870.

Extra line in "CMakeLists.txt" to include new file "gtg_interp.F90" from GTG was added. Per issue #840

@hsinmulin-NOAA You need to update the revision of the submodule post_gtg.fd to include gtg_interp.F90. @YaliMao-NOAA Could you help Hsin-mu on this?

@YaliMao-NOAA
Copy link
Contributor

@hsinmulin-NOAA I am sending instructions of how to update GTG revision number.

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA A sub version of gtg_interp.F90 is needed under sorc/ncep_post.fd. You may refer to an example:
https://github.com/NOAA-EMC/UPP/blob/develop/sorc/ncep_post.fd/gtg_mlmodel.F90

@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Dec 21, 2023 via email

Stub code for GTG protection but to make UPP public to work
@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Dec 21, 2023 via email

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA and @YaliMao-NOAA Now this PR is in a good shape for testing. Thanks for quick actions!

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA @YaliMao-NOAA From my UPP standalone test for RRFS GTG, there are to many printouts in runtime log (more than 60000 lines). Please work on limiting these debug information.

@WenMeng-NOAA
Copy link
Collaborator

The UPP standalone testing with RRFS NA 3-km data was conducted on WCOSS2. With 192 tasks (4nodes, 48task/node), the runtime of RRFS product generation was changed as:

Without new gtg dataset: 399 seconds
With new gtg dataset: 667 seconds

@hsinmulin-NOAA @YaliMao-NOAA @HuiyaChuang-NOAA Please see my test results at /u/wen.meng/ptmp/post_rrfs_2023060614-gtg.

The code change is to speed up and resolved the non-identical UPP outputs when compare  GTG on  & off (used or not used).
@YaliMao-NOAA
Copy link
Contributor

@hsinmulin-NOAA

  1. In MDLFLD.f, may you please change 477 back to 470? To run gtg_algo(), we only need to check EDPARM IDs to pull the trigger since EDPARM is kind of subset of CAT/MWT/CIT. I also added a line to the log history. Please compare my code on Cactus:
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/sorc/ncep_post.fd/MDLFLD.f

  2. To fix the issue that the last vertical level of CIT in AVIATI10.tm00 of RRFS GTG is marked wrongly, I have a fix on Cactus:
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/sorc/ncep_post.fd/MISCLN.f
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/parm/fv3lam_rrfs.xml
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/parm/postxconfig-NT-fv3lam_rrfs.txt

  3. Please add your comments of your code changes into the log history at the beginning of each Fortran code

  4. Please merge UPP code updates. Other PRs were merged after your PR was submitted.

1. update the comments
2.  fix the issue that the last vertical level of CIT in AVIATI10.tm00 of RRFS GTG per MISCLN.f
fix the issue that the last vertical level of CIT in AVIATI10.tm00 of RRFS GTG per change in MISCLN.f
@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Jan 24, 2024 via email

 switching GTG max to gtgx3 from gtgx2.
add comment for the change
@HuiyaChuang-NOAA
Copy link
Contributor

@hsinmulin-NOAA

  1. In MDLFLD.f, may you please change 477 back to 470? To run gtg_algo(), we only need to check EDPARM IDs to pull the trigger since EDPARM is kind of subset of CAT/MWT/CIT. I also added a line to the log history. Please compare my code on Cactus:
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/sorc/ncep_post.fd/MDLFLD.f
  2. To fix the issue that the last vertical level of CIT in AVIATI10.tm00 of RRFS GTG is marked wrongly, I have a fix on Cactus:
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/sorc/ncep_post.fd/MISCLN.f
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/parm/fv3lam_rrfs.xml
    /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/parm/postxconfig-NT-fv3lam_rrfs.txt
  3. Please add your comments of your code changes into the log history at the beginning of each Fortran code
  4. Please merge UPP code updates. Other PRs were merged after your PR was submitted.

@hsinmulin-NOAA make sure you pick up above fixes from Yali

@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Jan 24, 2024 via email

@HuiyaChuang-NOAA
Copy link
Contributor

Fixed and done per suggestion.

On Wed, Jan 24, 2024 at 9:58 AM HuiyaChuang-NOAA @.> wrote: @hsinmulin-NOAA https://github.com/hsinmulin-NOAA 1. In MDLFLD.f, may you please change 477 back to 470? To run gtg_algo(), we only need to check EDPARM IDs to pull the trigger since EDPARM is kind of subset of CAT/MWT/CIT. I also added a line to the log history. Please compare my code on Cactus: /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/sorc/ncep_post.fd/MDLFLD.f 2. To fix the issue that the last vertical level of CIT in AVIATI10.tm00 of RRFS GTG is marked wrongly, I have a fix on Cactus: /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/sorc/ncep_post.fd/MISCLN.f /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/parm/fv3lam_rrfs.xml /lfs/h2/emc/vpppg/noscrub/yali.mao/git/UPP.v17_hsinmu/parm/postxconfig-NT-fv3lam_rrfs.txt 3. Please add your comments of your code changes into the log history at the beginning of each Fortran code 4. Please merge UPP code updates. Other PRs were merged after your PR was submitted. @hsinmulin-NOAA https://github.com/hsinmulin-NOAA make sure you pick up above fixes from Yali — Reply to this email directly, view it on GitHub <#839 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALRSTCNBOILQUFIILO536CTYQEORVAVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYGMYDGMRWHA . You are receiving this because you were mentioned.Message ID: @.>

Great!

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA and @YaliMao-NOAA
I conducted the UPP standalone tests with 192 tasks on WCOSS2. By adding the new AVIATI dataset, the RRFS runtime increased from 399 seconds to 474 seconds. That performance looks good to me.
Please verify AVIATI10.tm00 file from my test at /u/wen.meng/ptmp/post_rrfs_2023060614-gtg on Cactus.

@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Jan 25, 2024 via email

@WenMeng-NOAA
Copy link
Collaborator

@hsinmulin-NOAA Please sync your branch with the latest UPP develop? We will process your PR soon. Thanks!

@hsinmulin-NOAA
Copy link
Contributor Author

hsinmulin-NOAA commented Jan 25, 2024 via email

@WenMeng-NOAA WenMeng-NOAA added the Ready for Review This PR is ready for code review. label Jan 25, 2024
@WenMeng-NOAA
Copy link
Collaborator

@FernandoAndrade-NOAA You may start the UPP RTs on Hera and Orion.

@WenMeng-NOAA WenMeng-NOAA added the No Baseline Change No baseline of the UPP regression tests are made. label Jan 25, 2024
@WenMeng-NOAA
Copy link
Collaborator

The UPP RTs were completed on WCOSS2 without changed results.

@FernandoAndrade-NOAA
Copy link
Collaborator

Hera UPP RTs completed successfully with no changes to results. Orion also shows no changes in most tests, however there was an issue between the test script and the gfs_pe test job submission causing it to end early while it sat in queue. I'm rerunning just the gfs test to be sure and will update shortly.

@FernandoAndrade-NOAA
Copy link
Collaborator

The UPP RTs were completed on WCOSS2 without changed results.

Orion RTs have completed as well. No changes in results, looks good from my side!

@WenMeng-NOAA
Copy link
Collaborator

This PR is ready for merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Baseline Change No baseline of the UPP regression tests are made. Ready for Review This PR is ready for code review. RRFS
Projects
None yet
6 participants