Development of a new unified cumulus convection scheme #56
Conversation
… into unified_conv
lisa-bengtsson
requested review from
grantfirl,
ChunxiZhang-NOAA,
dustinswales and
Qingfu-Liu
as code owners
|
@lisa-bengtsson Along with the new cu_unified_* files, I see that the cu_gf_* files are still in the repository. I think we can remove these? (This will make the deltas that appear in the PR a bit more vivid.) |
@dustinswales no there is no plan on removing the Grell-Freitas convection scheme from CCPP. The unified scheme will greatly diverge from that scheme (we have grand plans!), this is just a starting point. |
|
Lisa, should the bug-fix for the prognostic closure be first added to saSAS for running HR2 ? This unified convection scheme may take a while to develop and validate. |
Even though the unified scheme will be developed and tested over time, I think this PR could go in as a starting point so the other developers can add to the scheme, it does not break anything. |
@lisa-bengtsson Good to know, but what about the _pre _post routines that are currently identical? (I'm just trying to think of ways to potentially share code across convection schemes, rather than copying code and blazing your own trail for a new scheme) |
Yes, I didn't know what to do about those, because I had to create a new suite def file and thought it would look strange to have gf_pre and gf_post, while using unifed_deep and unified_shallow. I think ultimately GF, unified convection and the Tiedke scheme could share pre and post under a generic naming. Actually also SAS because the prog closure needs the fields computed in gf_pre and gf_post - and those are instead computed locally inside SAS right now. |
Maybe we chat sometime soon about a way forward to generalize and harmonize all these interstitials for the convection schemes. |
Yes, I think it is straightforward, but a lot of places needs to be updated. In particularly in the SDF files since there are so many. suite_FV3_GFS_2017_gfdlmp_noahmp.xml: samfdeepcnv And these are the ones using gf_pre/post: I would be open to make such changes, but I don't think it would fit in this PR. Perhaps a separate PR. What do you think? I also don't know about the RAS and CS convection schemes. |
|
@lisa-bengtsson I agree that it's beyond this PR to do the cleanup that is needed, and we'd probably need to reach out to the other convection scheme developers. As you said, it's not a ton of work, but it touches a lot of suites. But we do now have 6 convection parameterizations, four (GF, Unified, nTiedtke, CS) of which use similiarish _pre _post routines. The other two (SAS and RAS) don't require _pre/_post routines. |
Yes, although I would say SAS could use the similarish _pre _post as the four you mentioned because what those do is essentially to save and compute the tendency due to dynamics, and SAS requires this input now when using the prognostic closure, but it is currently computed inside the scheme. There is also some magic happening in the interstitial3 which I think could be combined with a more generic _pre _post. We could discuss it in the convection working group meeting if you think that is a good place for it. The developers join those, except Moorthi who retired. |
Gotcha. To me it seems like there are some easy cleanup opportunities here, and if the other developers agree, I say let's do it (As it's own PR) |
|
Sounds good Dustin. Another cleanup opportunity could be to get rid of some legacy SDF's. |
|
@lisa-bengtsson To help us get a sense of priority, how urgent is this PR? I see that it is relevant to HR2, but there is a list of physics developments that need to get merged for RRFS for the HWT spring experiment too. Is it OK to put this behind whatever RRFS physics development PRs come in? |
Looks like we'll have an HR2 meeting tomorrow, let me check what other PR's need to be prepared, and on the final timeline for HR2 and get back to you after that. Thank you for checking. |
|
@grantfirl @dustinswales following today's discussion I will move the parts pertaining to HR2 from this PR to a new PR. It would be nice if that can go after the RRFS PR's. Then this PR can go after the operational targeted PRs. |
|
@lisa Bengtsson - NOAA Federal ***@***.***> , thanks.
…On Fri, Apr 7, 2023 at 9:59 AM lisa-bengtsson ***@***.***> wrote:
@lisa-bengtsson <https://github.com/lisa-bengtsson>, did you resolve the
code conflicts listed in this pull request? Conflicting files
physics/sgscloud_radpre.F90 physics/sgscloud_radpre.meta
@lisa-bengtsson <https://github.com/lisa-bengtsson> Yes, please update
this PR branch to the latest ufs/dev when you get a chance. This should
resolve the conflicts.
@Qingfu-Liu <https://github.com/Qingfu-Liu> @grantfirl
<https://github.com/grantfirl> yes, I will work on this today, I have to
do a bit of a manual merge with the sas convection routines as I pulled
those updates out from this PR to the previous PR.
—
Reply to this email directly, view it on GitHub
<#56 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTS6UTB3Q5S4BV6K7EVOL3XAAMSJANCNFSM6AAAAAAWCVNJKY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi Grant, thank you for taking a close look. The code you commented on is inherited from either the GF or the SAS scheme, I agree that all of them should be addressed. Is there anything that should be done in this PR, or can we commit a "clean up" PR after @haiqinli has added his updates to this scheme? As you know there is some room for improvement in the pre/post of all convection schemes, and we could combine such cleanup with the suggestions you and others have given here. |
@lisa-bengtsson Ya, I should have been clearer, my comments are for future reference for a cleanup PR. As long as the code is where you all want it for the initial commit, I think it's fine to commit what is here, perhaps simultaneous to adding an official issue to fix some of these things in the future so that they're not forgotten. |
|
@dustinswales @Qingfu-Liu Care to re-review? Don't feel obligated. |
|
@grantfirl Thanks, I created a new issue here: #63 |
|
@grantfirl I had reviewed this PR#56 last Friday, and approved the pull request |
|
@lisa-bengtsson Given that #64 was just added, is this where the unified convection and GF schemes will diverge? Or, do you want to pull in the changes from that PR into the new unified scheme? |
|
Thank you for checking @grantfirl @haiqinli will add the updates also to the unified code. These are the current next steps: Short-term next steps:
Further development:
|
|
@dustinswales I'm going to be out until about 2 PM ET today, so if this needs to get merged today, please watch for the queue and do so when requested. |
|
All weather model regression tests are done at ufs-community/ufs-weather-model#1669. @grantfirl can you merge this pr? |
This addresses issue: ufs-community/ufs-weather-model#1694
This PR is the first of several to function as a starting point for the unified convection scheme.
The following are the updates are in this PR (Coordinated between Lisa Bengtsson, Georg Grell, Jongil Han and Haiqin Li) :