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

Radar-derived microphysics temperature tendencies similar to operational HRRR #823

Conversation

SamuelTrahanNOAA
Copy link
Collaborator

@SamuelTrahanNOAA SamuelTrahanNOAA commented Dec 30, 2021

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

@grantfirl
Copy link
Collaborator

@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
Copy link
Collaborator

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?

Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator

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.

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
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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

exit
enddo
if_radar: if(itime<=num_dfi_radar) then
radar_k: do k=3,levs-2
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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
Copy link
Collaborator

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?

Copy link
Collaborator Author

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
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator

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.

@SamuelTrahanNOAA
Copy link
Collaborator Author

@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.

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.

@grantfirl
Copy link
Collaborator

grantfirl commented Jan 4, 2022

@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.

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.

@SamuelTrahanNOAA
Copy link
Collaborator Author

@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.

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.

@grantfirl
Copy link
Collaborator

grantfirl commented Jan 4, 2022

@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.

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.

@SamuelTrahanNOAA
Copy link
Collaborator Author

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?

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.

You're saying that the DA and workflow developers need this code in the develop repositories first in sequence and not simultaneous to this?

The RRFS developers need the ufs-weather-model to stop breaking their configuration to continue that work.

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", adding undocumented code like this can be pretty confusing.

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.

@yangfanglin
Copy link
Collaborator

@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.

@grantfirl
Copy link
Collaborator

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.

enddo
if_radar: if(itime<=num_dfi_radar) then
do_cap_suppress = 1
cap_suppress_j = dfi_radar_tten(:,1,itime)
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.)

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

@grantfirl
Copy link
Collaborator

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.

@SamuelTrahanNOAA
Copy link
Collaborator Author

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.

Copy link
Collaborator

@grantfirl grantfirl left a 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.

physics/GFS_MP_generic.F90 Outdated Show resolved Hide resolved
physics/GFS_MP_generic.F90 Outdated Show resolved Hide resolved
physics/GFS_MP_generic.F90 Outdated Show resolved Hide resolved
physics/GFS_MP_generic.F90 Outdated Show resolved Hide resolved
physics/cu_gf_driver.F90 Outdated Show resolved Hide resolved
physics/cu_gf_driver.F90 Outdated Show resolved Hide resolved
physics/cu_gf_driver.F90 Outdated Show resolved Hide resolved
@grantfirl
Copy link
Collaborator

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.

@grantfirl grantfirl self-requested a review January 7, 2022 16:00
@SamuelTrahanNOAA
Copy link
Collaborator Author

I've made the many minor code changes Dom asked for. (Look in the resolved conversations for details.)

Copy link
Collaborator

@climbfuji climbfuji left a 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 ...

physics/GFS_MP_generic.F90 Outdated Show resolved Hide resolved
Copy link
Collaborator

@climbfuji climbfuji left a 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.

@grantfirl
Copy link
Collaborator

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.

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Jan 7, 2022 via email

@SamuelTrahanNOAA
Copy link
Collaborator Author

SamuelTrahanNOAA commented Jan 7, 2022

There might be some misunderstanding, but I think Sam's PR and Moorthi's PR are still two separate PRs.

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.

@gthompsnWRF
Copy link
Collaborator

@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.

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.

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