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

32-bit physics in FV3_RAP #930

Merged
merged 26 commits into from
Jul 19, 2022

Conversation

SamuelTrahanNOAA
Copy link
Collaborator

@SamuelTrahanNOAA SamuelTrahanNOAA commented May 30, 2022

This adds support for 32-bit physics to the FV3, based on prior work on the Neptune model. The 64-bit physics should not change results. See #923 for past conversation and ufs-community/ufs-weather-model#1215 for the top-level PR.

About half of these changes come from @DusanJovic-NOAA, and a little bit from @climbfuji and @grantfirl. The work is based on a similar project by @michalakes in the NRL Neptune model, which also uses CCPP.

Issues

ufs-community/ufs-weather-model#1288

Dependencies

earth-system-radiation/rte-rrtmgp#180 this was already merged

Copy link
Collaborator

@ChunxiZhang-NOAA ChunxiZhang-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@AnningCheng-NOAA
Copy link
Collaborator

Looks fine. Approved.

@@ -10823,6 +10823,7 @@ real*8 function zolri(ri,za,z0,zt,zol1,psi_opt)
real (kind=kind_phys) :: x1,x2,fx1,fx2
integer :: n
integer, parameter :: nmax = 20
real(kind=kind_phys) rphys_temp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SamuelTrahanNOAA for clarity, can you change this to something like rphys_dummy or rphys_temporary? at first, I was trying to figure out how this was a temperature.

it might be even more clear to name this variable zolri_iteration and the one below as zolrib_iteration or something similar.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better names. I just made that change.

@SamuelTrahanNOAA
Copy link
Collaborator Author

I've merged #1192 to this branch, and both the hera.intel and hera.gnu tests still pass. The results of the new 32-bit physics tests don't change, nor do the existing tests. These tests also included the variable name change suggested in one of the comments.

@SamuelTrahanNOAA SamuelTrahanNOAA merged commit 12c115e into NCAR:main Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants