-
Notifications
You must be signed in to change notification settings - Fork 159
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
sync with head of NOAA-EMC UPP develop #845
sync with head of NOAA-EMC UPP develop #845
Conversation
@SamuelTrahanNOAA When you submit ufs-weather-model PR with upp submodule updating to the latest commit from UPP repository, all UPP control files post-xconfig*.txt (under ufs-weather-model/test/parm) need to be recreated for UFS RTs. |
I need your help with that. I'm uncertain of which txt file in tests/parm corresponds to which file in FV3/upp/parm. Also, I'm wondering if any need modification before copying to tests/parm. |
These three files from tests/parm don't exist in FV3/upp/parm:
This does exist, but it's the wrong one:
The tests/parm version has no synthetic satellite brightness temperature fields, while the FV3/upp/parm one does. It isn't the |
These are simple versions of UPP control files for inline post testing for gfs, rffs and hafs. I will find corresponding xml files and provide you updated txt files later. |
@WenMeng-NOAA - As mentioned in this comment, I need help debugging the inline post.
You can find the output here: LOG: /scratch1/NCEPDEV/stmp2/Samuel.Trahan/FV3_RT/rt_3037467/gnv1_nested_intel.log Many ranks give the same error many times, but it's always the same error. Code is here: ufs-community/ufs-weather-model#2326 It only works on Hera because only Hera has g2tmpl 1.12.0 in the same Spack Stack as ufs-weather-model prerequisites. |
There are a multitude of warnings coming from a library when the latest UPP is used.
@WenMeng-NOAA has tracked this down to the UPP pull request NOAA-EMC/UPP#929. The warnings vanish if you revert the changes to the postxconfig files (or rather, to the post_avblflds.xml used to generate them). The warnings don't seem to have an effect on the output.
EDIT: We have a bug fix from the UPP side that eliminates these error messages. It is in this PR. |
Good news, everyone! We have a fix for the "255 not found in table 4.5" messages! That means this PR can continue review without thousands of error messages from g2tmpl. |
I'd like @DusanJovic-NOAA @WenMeng-NOAA and @EricJames-NOAA to look at this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks for working on this.
Add more upp updates
@zhanglikate - Your message was garbled, so I don't know what you want. Please do one of these:
|
Hi Sam,
Where is the "suggested change” feature? I added comments in the place as “suggested change”. Thanks.
… On Jul 23, 2024, at 5:33 PM, Samuel Trahan (NOAA contractor) ***@***.***> wrote:
@zhanglikate <https://github.com/zhanglikate> - Your message was garbled, so I don't know what you want.
Please do one of these:
Use the github "suggested change" feature, or
Open a PR to this branch with your changes.
—
Reply to this email directly, view it on GitHub <#845 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APJPDRBOUWHXP7NA67PYIZDZN3R23AVCNFSM6AAAAABJG45DOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBWGUYDENRRGE>.
You are receiving this because you were mentioned.
|
@SamuelTrahanNOAA Could you update the upp harsh to the latest commit @81b38a8 from the UPP repository? |
Yes. It matches that hash now. |
Could I get some reviews by people "in the know?"
|
The upp submodule update and inline post interface "post_fv3.F90" look good to me. |
Github workflow failed:
|
That looks a lot like an out-of-date library. Maybe @RatkoVasic-NOAA can comment? |
The github workflow gets its UPP dependencies for spack from this line in - upp@develop The In other words, FV3 is getting its UPP dependencies from the spack repository instead of the UPP repository. |
@DusanJovic-NOAA Please check if g2/3.51 and g2tmpl/1.13.0 are used in CI. |
It's using a cached version of spack, not the latest build. There should be an update to |
Actually I think we should ci/spack.yaml, because the cache key is defined based on hash of spack.yaml. So in order to detect potential changes in any of the libraries we should explicitly list every single library and its version , and not use 'upp@develop' which basically 'hides' the changes in upp dependencies. upp@develop can mean different things at different time. Basically all dependencies must be versioned. And instead of just saying upp, I prefer to have a explicit list of libraries with explicit versions. Try to change spack.yaml in your branch and see if it triggers spack cache rebuild. |
I've updated spack.yaml, and it is rebuilding. There were library version conflicts, so I chose the latest version of the two. What worries me is this one:
Using the |
Yes. All libraries should be versioned, develop is ambiguous. I would use versions from your ufs-weather-model branch ufs_common.lua from modulefiles directory, with updated versions of g2 and g2tmpl. Also, fv3atm does not need sigio, sfcio, nemsio and wrf-io. Those are required by the upp executable, which we are not building here. |
@DusanJovic-NOAA - Could you check the spack.yaml? It should match your specifications now. |
The spack.yaml looks good. And I see workflow finished successfully. Thank you for fixing this. |
I made an issue for the spack.yaml troubles: It'll be automatically closed when this PR is merged. |
@jkbk2004 testing is finished on WM PR 2326. Feel free to merge this PR. |
* send CCPP ebu_smoke to UPP ebb * upp: remove GSD_NC synonyms and rename GSD_NC fields instead * update upp hash: g2, g2tmpl, etc. * upp: correct pressure levels for hafs-ar postxconfig files * bugfix: fixed_sfc2_type defaults to fixed_sfc1_type * Add aerosol fix from Li Pan. --------- Co-authored-by: Wen Meng <Wen.Meng@noaa.gov>
Description
This updates the upp submodule to the head of NOAA-EMC UPP's develop branch.
Also, the spack.yaml has explicit UPP dependencies (g2, g2tmpl, etc.) and all libraries have exact version numbers. These version numbers match the top of ufs-weather-model's develop branch for hercules.
Issue(s) addressed
Testing
How were these changes tested?
Tested in the HAFS-AR variant of the HAFS workflow in a large stationary regional domain over the north-east Pacific. Values looked reasonable. Also ran UPP and ufs-weather-model regression tests.
What compilers / HPCs was it tested with?
Hera intel and Hera gnu.
Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
The ufs-weather-model regression tests already test the inline post. However, I added three new number concentration fields to the ufs-weather-model gnv1_nested regression test.
Have the ufs-weather-model regression test been run? On what platform?
The ufs-weather-model regression testing is in progress now. Baseline generation on Hera succeeded.
UPP's own regression tests passed. That tests the offline post (running UPP outside FV3).
Will the code updates change regression test baseline? If yes, why? Please show the baseline directory below.
Results of many tests should change due to updating the UPP hash.
Please commit the regression test log files in your ufs-weather-model branch
Will do.
Dependencies
None.