-
Notifications
You must be signed in to change notification settings - Fork 105
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
Conversation
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 |
The GitHub issue link is #820 <#820>
The detail changelog about " MISCLN.f" can be found at
hsinmulin-NOAA@3d47e0b
…On Mon, Dec 11, 2023 at 4:45 PM Fernando Andrade - NOAA < ***@***.***> wrote:
Hi @hsinmulin-NOAA <https://github.com/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
—
Reply to this email directly, view it on GitHub
<#839 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRSTCPJV3ULJ4KYXAYLXD3YI55F3AVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJQHEZTQNJUGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@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? |
Hi, Yali
Let me create an issue related to it and commit it later.
There is an interesting issue for " CMakeLists.txt " to include it or not
that I have mentioned in the GTG update discussion. I need to clear why it
exists before we get it done.
…On Fri, Dec 15, 2023 at 10:50 AM YaliMao-NOAA ***@***.***> wrote:
@hsinmulin-NOAA <https://github.com/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?
—
Reply to this email directly, view it on GitHub
<#839 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRSTCN2BLW4ZY3R45K2MLDYJRWSPAVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJYGA4TGOJTHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@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. |
@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.
Extra line in "CMakeLists.txt" to include new file "gtg_interp.F90" from GTG was added. |
@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? |
@hsinmulin-NOAA I am sending instructions of how to update GTG revision number. |
@hsinmulin-NOAA A sub version of gtg_interp.F90 is needed under sorc/ncep_post.fd. You may refer to an example: |
The version of the GTG has been updated per instructions
…On Thu, Dec 21, 2023 at 4:22 PM YaliMao-NOAA ***@***.***> wrote:
@hsinmulin-NOAA <https://github.com/hsinmulin-NOAA> I am sending
instructions of how to update GTG revision number.
—
Reply to this email directly, view it on GitHub
<#839 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRSTCPUSL54X34GTTB5YTTYKSSAFAVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRWHE2DIMRYGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Stub code for GTG protection but to make UPP public to work
Added the sub version per request.
…On Thu, Dec 21, 2023 at 5:00 PM WenMeng-NOAA ***@***.***> wrote:
@hsinmulin-NOAA <https://github.com/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
—
Reply to this email directly, view it on GitHub
<#839 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRSTCL33ESYJ6X75W3RHQTYKSWRBAVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRWHE4DGNBVGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@hsinmulin-NOAA and @YaliMao-NOAA Now this PR is in a good shape for testing. Thanks for quick actions! |
@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. |
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:
@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).
|
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
Modification per suggestion has been done and tested.
…On Wed, Jan 10, 2024 at 1:41 PM YaliMao-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.
—
Reply to this email directly, view it on GitHub
<#839 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRSTCMYQWQOSIVWXG5NXJ3YN3OFVAVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBVGQYTQMZUGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
switching GTG max to gtgx3 from gtgx2.
add comment for the change
@hsinmulin-NOAA make sure you pick up above fixes from Yali |
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! |
@hsinmulin-NOAA and @YaliMao-NOAA |
The output are identical to the sensitivity test case and all look good to
me.
…On Wed, Jan 24, 2024 at 8:02 PM WenMeng-NOAA ***@***.***> wrote:
@hsinmulin-NOAA <https://github.com/hsinmulin-NOAA> and @YaliMao-NOAA
<https://github.com/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.
—
Reply to this email directly, view it on GitHub
<#839 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRSTCNN6YRNIJBBIK7AK53YQGVILAVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBZGE3TINJSGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@hsinmulin-NOAA Please sync your branch with the latest UPP develop? We will process your PR soon. Thanks! |
The branch code has been synced and has been compared with the previous
version. The change is expected
It is done.
…On Thu, Jan 25, 2024 at 12:23 PM WenMeng-NOAA ***@***.***> wrote:
@hsinmulin-NOAA <https://github.com/hsinmulin-NOAA> Please sync your
branch with the latest UPP develop? We will process your PR soon. Thanks!
—
Reply to this email directly, view it on GitHub
<#839 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRSTCOQ3DHC77ZZ3Q3YDN3YQKIKVAVCNFSM6AAAAABAQJVPR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJQGY3DGMRUGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@FernandoAndrade-NOAA You may start the UPP RTs on Hera and Orion. |
The UPP RTs were completed on WCOSS2 without changed results. |
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. |
Orion RTs have completed as well. No changes in results, looks good from my side! |
This PR is ready for merging. |
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)