-
Notifications
You must be signed in to change notification settings - Fork 147
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
Radar-derived microphysics temperature tendencies similar to operational HRRR #823
Radar-derived microphysics temperature tendencies similar to operational HRRR #823
Conversation
…s-community-20211220
…-ufs-community-20211220
@SamuelTrahanNOAA In either this PR or one of the upstream ones, it would be great to have some documentation for how to use this new functionality along with example use cases and/or limitations. |
|
||
! add radar temp tendency | ||
! there is radar coverage | ||
gt0(i,k) = save_t(i,k) + ttend*dtp |
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.
I have reservations about this functionality being added to this interstitial scheme. Other than one interstitial scheme that updates the state variables after the process-split section of physics is completed, I don't think that any others update the state. In general, they are supposed to have "glue code", not necessarily algorithms that actually affect the state variables. Why could this functionality not be added to a new scheme, especially if it is supposed to be used with HRRR-based suites at this point?
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.
In other words, this is sorta a physics parameterization itself, "helping out" microphysics. Why should it not exist as its own scheme?
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.
It can't because the calcpreciptype has to use the actual microphysics temperature tendencies, or it'll get meaningless values. Somehow it has to go after that calculation, but before the later code.
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.
OK, I see what you mean. It's a bit messy and goes against precedent, but given how the code is organized now, I don't see an alternative if the constraint you mentioned is legit.
physics/GFS_MP_generic.F90
Outdated
integer, intent(in) :: imp_physics, imp_physics_gfdl, imp_physics_thompson, imp_physics_mg, imp_physics_fer_hires | ||
logical, intent(in) :: cal_pre, lssav, ldiag3d, qdiag3d, cplflx, cplchm | ||
integer, intent(in) :: index_of_temperature,index_of_process_mp | ||
|
||
real(kind=kind_phys), intent(in) :: fh_dfi_radar(5), fhour |
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.
I don't understand why fh_dfi_radar is hard-coded to be size 5 array (here and in GFS_typedefs). Is there some significance to 5 time intervals for this functionality?
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.
The system is designed to use either 4 time intervals or 1. The number 5 comes from the end points before and after each interval. "|1|2|3|4|" has five "|" in it. I could put that 4 in some constant, if you prefer.
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.
OK, well that should be explained in comments or something. No need to define a new variable for 4, IMO, unless someone will want to use a different number of intervals, but uncommented hard-coded integers are almost always confusing to readers trying to understand code after it's written.
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.
Unfortunately, you responded too late: I already made a new constant for 4. The code makes a lot more sense that way. You'll see in a few hours when I push the changes.
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.
There is a new variable with standard name maximum_number_of_radar_derived_temperature_or_convection_suppression_intervals which replaces the number 4
physics/GFS_MP_generic.F90
Outdated
exit | ||
enddo | ||
if_radar: if(itime<=num_dfi_radar) then | ||
radar_k: do k=3,levs-2 |
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.
Why can these tendencies only be applied from k=3, levs-2?
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.
I'm duplicating the original WRF formulation. The data assimilation turns off the tendencies (using -20 values) near the top and bottom of the model. This is just a safeguard against the DA forgetting to do that.
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.
Also, level 1 contains the convection suppression information for the GF scheme, not temperature tendencies. Applying that data as temperature tendencies would cause bizarre and unwanted effects.
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.
What was at level 1 is now in a new variable with standard name radar_derived_convection_suppression.
! gt0(i,k) = save_t(i,k) + dfi_radar_tten(i,k,itime) | ||
! endif | ||
ttend = dfi_radar_tten(i,k,itime) | ||
if_active: if (ttend>-19) then |
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.
I don't understand the hard-coded test for >-19. What if a user specifies radar_tten_limits(1) < -19?
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.
Such a large value would be meaningless. The fill value in the data assimilation code is -20, and that is what the "if" block tests for.
ttend = dfi_radar_tten(i,k,itime) | ||
if (ttend>-19) then | ||
if(idtend_radar>0) then | ||
dtend(i,k,idtend_radar) = dtend(i,k,idtend_radar) + (gt0(i,k)-save_t(i,k)) * frain |
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.
Aren't you saving microphysics tendency + radar tendency in the radar tendency slice here? Won't it be possible for both microphysics and radar tendencies to be nonzero simultaneously?
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.
The radar tendencies override the microphysics tendency; that is the purpose of this PR. In each gridpoint, you can have one, or the other, not both.
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.
At the point where this interstitial scheme is called, microphysics tendencies have already been applied since they are being used as time-split in every CCPP suite that I'm aware of. Unless I'm mistaken, this is only replacing the microphysics tendency diagnostic, not the one that actually gets applied to the state. Wouldn't one need to insert code in every microphysics scheme to bypass it when you wanted to apply the radar tendencies?
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.
No. In the RAP and HRRR suites, only the thompson scheme is run between GFS_MP_generic_pre and _post.
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.
OK, so the Thompson scheme is called and updates gt0. Then, this is called and replaces the previously calculated gt0 if a radar tendency is present.
This feature requires modifications to the data assimilation system and workflow. I don't know if those have been PR'd yet. Until then, there isn't much to document other than the meaning of namelist variables. |
If companion PRs are necessary for this PR, we should probably wait to merge this until the other ones are ready, right? Also, companion PRs for other parts of the UFS system should be mentioned (or linked) to this PR so that people can follow the entire feature and its code changes. |
No! There is no need to put such pain on developers just to complete a documentation website. The RRFS parallels keep having to hold back updates because the community repositories keep breaking support for their configuration. They NEED these changes in the community repositories. |
My question was asked to try to understand where this PR should fit in the UFS commit queue -- whether there will be other PRs immediately forthcoming that are linked to this functionality. As these PRs are now, this functionality only can be exercised with special test input data for the purpose of RTs, right? You're saying that the DA and workflow developers need this code in the develop repositories first in sequence and not simultaneous to this? As far as documentation, I'm thinking of physics developers who are wondering how to populate the tendency terms if they want to use this. If everybody who wants to use this already has that knowledge, then I guess there is no need to document anything a priori, but from the point of view of a physics developer who sees this functionality coming "out of left field" and not being pointed to code changes that explain where the terms are generated, adding undocumented code like this can be pretty confusing. |
The code and scripts are in NOAA-GSL's repositories. I don't know when they're planned to go to the community repository, but it has to be after this PR.
The RRFS developers need the ufs-weather-model to stop breaking their configuration to continue that work.
I will have to discuss this with @hu5970 and others, and see what sort of documentation we can put together. The best I can do is explain what is in the input variables and the meaning of namelist variables. Without the ability to generate the data, that will be of limited use. We cannot wait for the PR though because, as I said several times, breaking this configuration is holding up RRFS work. |
@hu5970 MIng, Is there an existing HRRR publication or notes that documented this radar-derived microphysics temperature tendencies ? Even though this function is controlled by namelist options in the CCPP codebase, it is still a bit concerning for CCPP development in general if microphysics temperature tendency are modified not following the first physics principles and changes are made to temperature but not to cloud hydrometers. |
I also agree with @yangfanglin about the choice to replace only temperature tendencies from microphysics from radar and not hydrometeors. It seems like this should somehow affect radiation too. |
physics/cu_gf_driver.F90
Outdated
enddo | ||
if_radar: if(itime<=num_dfi_radar) then | ||
do_cap_suppress = 1 | ||
cap_suppress_j = dfi_radar_tten(:,1,itime) |
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.
@SamuelTrahanNOAA Based on your comment about the first level of dfi_radar_tten, does that mean that different slices of this array have different physical meaning? I.e. the first level is not actually a temperature tendency from radar? If so, I'm pretty sure that the CCPP rules frown on that. Why not define a new variable and leave dfi_radar_tten to contain only what's advertised?
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.
There's a limitation inside the data assimilation package that prevents that. (Sorry, I don't know the details.)
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.
Just because the data comes in to the model like that, why can't it be separated out in GFS_typedefs so that the CCPP/physics sees an array that contains what it claims? For purposes of restart/diagnostics, maybe the convection suppression information can be put back in that array, but hiding information in variables goes against the transparency motivations of the CCPP.
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.
I think a reasonable temporary solution is to give it its own array, breaking the DA code, and fix the DA code later. No workflow is using the suppression information yet, and the issue can be fixed at a workflow level if the DA code can't be fixed.
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.
I'm OK with that solution. I appreciate the attempt to save a bit of memory/storage by the DA folks, but in this instance, I think code clarity/transparency is likely more important.
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.
What was at level 1 is now in a new variable with standard name radar_derived_convection_suppression.
FYI, @SamuelTrahanNOAA Jun has this PR tentatively scheduled for merge this Friday, 1/7, so hopefully that will work for RRFS devs, assuming any changes due to the review are finished by then. |
The changes discussed here, so far, will not impact their work since the convection suppression isn't in use. Otherwise, it's just internal CCPP bookkeeping changes and code clean-up so far. |
…-ufs-community-20211229
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.
Thanks @SamuelTrahanNOAA. I was waiting to approve until I had a chance to make sure that the new variable names are consistent with the rules, but I can always do that in a followup PR that only changes metadata if necessary.
Thanks @climbfuji for catching the argument array dims. I always seem to overlook that. @SamuelTrahanNOAA I just communicated to Jun to hold off for a bit while Dom's comments are addressed. |
I've made the many minor code changes Dom asked for. (Look in the resolved conversations for details.) |
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.
One more tiny change ...
…r-tten-ufs-community-20211229
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.
Sam's changes look good, and Moorthi's as well. I approve based on the assumption that Sam was told to combine those two PRs.
I thought that Sam was anticipating this PR being merged after Moorthi's, but I don't think that they are supposed to be combined, unless I'm out of the communication loop. |
There might be some misunderstanding, but I think Sam's PR and Moorthi's PR
are still two separate PRs.
…On Fri, Jan 7, 2022 at 1:43 PM Grant Firl ***@***.***> wrote:
I thought that Sam was anticipating this PR being merged after Moorthi's,
but I don't think that they are supposed to be combined, unless I'm out of
the communication loop.
—
Reply to this email directly, view it on GitHub
<#823 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI7D6TIHPBATWOWX6UY34LLUU4X6PANCNFSM5K7UMQRQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
My understanding is this: Moorthi's PR goes in first. Sam merges Moorthi's PR into his branch now, and tests the two together. Sam's PR goes in second. |
As the microphysics guy here, I have always had concerns about this temperature tendency adjustment. In effect, I understand the point of releasing latent heat, however, I've always been concerned that the heat addition could actually lead to the opposite effect with suppressing further development of cloud/precip since it could act to "stabilize" the vertical temperature profile rather than destabilize. The most obvious reason why models cannot create storms where they are found in reality is the lack of proper vertical motion forcing. And that is probably more often linked to a lack of proper lower-level convergence than in reality. So I have always wanted to affect the divergence/convergence winds rather than mess with heating terms and microphysics, because those two will adjust automatically once the added convergence can initiate the storms in the first place. I think it was Andrew Molthan (NASA) who did some wind nudging work towards this idea. We cannot really just add either hydrometeors or water vapor, because then we will be introducing a mass imbalance (unless we take the Qvapor from somewhere else in the system). Alas, this is pretty much the ultimate reason I left behind the microphysics work of ~20 years to pursue the satellite data assimilation at JCSDA. It's time to advance satellite DA with clouds better. I hope to make an impact in this arena. |
…-ufs-community-20211229
This implements a feature of the operational HRRR, radar-derived microphysics temperature tendencies applied in the first N minutes of the forecast to improve clouds in the first few hours. This has been tested for several weeks of forecasts and found to improve clouds. Collaborators on the DA and workflow development of this are @AmandaBack-NOAA @Ruifang-Li @hu5970
Also in here is convection suppression similar to the operational RAP. That is experimental, as it has only undergone technical testing and one case study. The code will warn you if you try to use it.
See issue at ufs-community/ufs-weather-model#985
fv3atm PR is at NOAA-EMC/fv3atm#457
ufs-weather-model PR is at ufs-community/ufs-weather-model#986