-
Notifications
You must be signed in to change notification settings - Fork 150
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
Fix NoahMP precip rates #365
Conversation
…P_generic_post_run from sfc_noahmp_pre; sfc_noahmp_pre no longer needed
I would like to pull this into #366 and merge it into dtc/develop (provided that all tests except the noahmp test pass). Please let me know if that's ok. |
I have only compiled and run with the SCM with this code. The associated changes in the SCM and FV3 are done, except for the associated change in GFS_physics_driver for the non-CCPP physics. This has also not been OKed by Helin Wei yet. But, if you want to test it, go ahead. I'll go ahead and make the changes to GFS_physics_driver and commit to fv3atm tonight real quick. |
This can wait until tomorrow, no problem! Thanks for making those changes. If we can show that it makes a noticeable difference (and especially makes noahmp less dry), then that's great. |
This PR has been pulled into #366 |
Associated PRs: Being tested as part of #366 by @climbfuji |
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.
Please do not merge this PR as it will be merged automatically with #366.
This and associated PRs address issue #319
This moves the calculation of precipitation rates needed by NoahMP LSM to GFS_MP_generic_post_run from sfc_noahmp_pre. Since this was the only thing done in sfc_noahmp_pre, it is no longer needed.
Note: As commented in the code and as noted by @climbfuji, there may be an issue with the calculation of these precipitation rates since they rely on liquid water equivalent thickness variables. From looking at GFS_physics_driver, it looks like Diag%rain,rainc are on the dynamics timestep whereas Diag%snow,ice,graupel are on the physics time step. This may not be consistent across all microphysics schemes. Of course, this issue is hidden as long as dtp=dtf (frain=1). This could definitely benefit from some cleanup if dtp is ever expected to be different than dtf.