Update UPP (2026-01)#1060
Conversation
|
fyi @WenMeng-NOAA |
|
@NickSzapiro-NOAA Thanks for working on upgrading upp hash. |
|
Could we get reviews on this PR so that we can process WM parent PR 3072? |
|
@WenMeng-NOAA @AlysonStahl-NOAA I see this unit test failure in GCC / build_fv3atm (-D32BIT=ON, 12, openmpi): |
@NickSzapiro-NOAA Should we hold off on processing WM PR 3072, or has this been resolved (or are we moving forward anyway?)? |
|
I would wait until we hear back @gspetro-NOAA, if possible. Please feel free to move onto another PR |
|
@NickSzapiro-NOAA At first glance, this appears to be related to small ulp differences in LOG() and EXP() across environments. I'll probably have to make some small changes to the test and possibly others to make sure they pass consistently. |
|
@AlysonStahl-NOAA So would you say that these results are normal/expected, and it's okay to proceed with the PR? Or do you think some more checking is necessary to be certain there's no error/issue? |
|
@gspetro-NOAA In my opinion, yes. The same unit tests are passing in UPP's CI, which is where we intend to run them. I don't believe there have been any fundamental changes to the subroutine being tested, but if there were then that would be cause for concern since that is what the tests are meant to catch. I'm not too familiar with this repository, just that it uses UPP. Do you intend on running UPP's unit tests with the fv3atm tests? We would like the tests to pass on every environment we intend to use UPP, so it's good we caught it here. The test should be fixed, but the issue is just that the difference between the calculated and expected values in the test fall outside a somewhat arbitrarily selected tolerance. |
|
@AlysonStahl-NOAA You know better than me, but the difference in values is ~O(0.01), which does not seem too small. Gemini AI does flag an issue that the test should allocate and fill like sorc/ncep_post.fd/ALLOCATE_ALL.f instead of nlevs In the test, so I don't know where values for ALPINT(I,J,61), ZINT(I,J,61) are coming from |
|
Ok, I see what's happening now. Thanks for catching that. I'm fixing it right now. |
|
@AlysonStahl-NOAA Thanks for submitting the fix in UPP. |
|
Tests pass now. Thanks @AlysonStahl-NOAA and UPP team! |
|
@NickSzapiro-NOAA The CCPP PR has been merged. You can revert .gitmodules and update the CCPP hash to b73b97b. |
|
Thanks! git clone isn't working for me at the moment ... "Could not resolve host: github.com" |
|
Testing for WM PR 2072 has completed successfully, CCPP PR 343 has been merged, and Nick has reverted .gitmodules and updated the CCPP hash. This PR can be merged. |
|
@DusanJovic-NOAA Could you merge this PR? |
Description
Update UPP to top of develop, including EE2 fixes for SWRF, SDEN to reproduce when threading. Also adds UPP unit testing. Bit-for-bit
Issue(s) addressed
Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)
Testing
How were these changes tested? UPP commit testing and UFS RTs on Ursa. Answers are bit-for-bit
See ufs-community/ufs-weather-model#3072
Dependencies
Commits already merged in UPP