add semi-Lagrangian sedimentation of graupel into Thompson MP#770
Conversation
RuiyuSun
requested review from
climbfuji,
grantfirl and
llpcarson
as code owners
climbfuji
left a comment
climbfuji
left a comment
There was a problem hiding this comment.
This looks good to me, will approve after the regression testing is done. To confirm, this will not change the answers of the existing regression tests, because the feature is turned off by default?
|
This is added as an option. This will not change the results when turned
off. A RT test of control_thompson was performed and passed on HERA
…On Fri, Nov 5, 2021 at 10:42 AM Dom Heinzeller ***@***.***> wrote:
***@***.**** commented on this pull request.
This looks good to me, will approve after the regression testing is done.
To confirm, this will not change the answers of the existing regression
tests, because the feature is turned off by default?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#770 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKFOCLDAVP6SMJRJMJYORNDUKP3N3ANCNFSM5HN4HQXA>
.
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>.
--
Ruiyu Sun, PhD
*IMSG* at NOAA/NWS/NCEP/EMC
5830 University Research Ct., Rm. 2097
College Park, MD 20740
***@***.*** ***@***.***>
301-683-3787
|
gthompsnWRF
left a comment
gthompsnWRF
left a comment
There was a problem hiding this comment.
Very minor changes to names requested, otherwise, I'm fine with this.
| nr_tmp(:) = nr(:) | ||
| call nislfv_rain_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1) | ||
| call nislfv_rain_ppm(kte,dzq,vtnrk,nr,vtr,dtcfl,R2) | ||
| call nislfv_precip_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1) |
There was a problem hiding this comment.
While you changed "rain" to "precip" I really don't understand "nislfv" nor "ppm" and would appreciate something better. These are jumbled letters to me and others. Got anything else?
There was a problem hiding this comment.
nislfv and ppm are described in the description section of the routine.
nislfv= non-iteration semi-Lagrangain forward advection
ppm = 2nd order interpolation with monotonic piecewise parabolic method
There was a problem hiding this comment.
Good to know it's documented somehow, but I see it as an improvement to use something like subroutine semi_lagrange_precip (or anything to give a clue by name in the calling routine. It's great to have the more full descript inside the routine.
There was a problem hiding this comment.
What do you suggest using? Any idea?
There was a problem hiding this comment.
@Songyou184 What do you think about the names?
There was a problem hiding this comment.
What do you suggest using? Any idea?
I did give you my suggested name of subroutine in my comment. It is: semi_lagrange_precip or perhaps better is semi_lagrange_sedim
|
The latter, semi_lagrange_sedim, is my preference. I wonder if my previous
comments in git environment were delivered correctly.
In addition to this naming, in 1st line of the description in the
subroutine, ....for cloud is to be replaced with ....for falling
hydrometeors. Also, the corresponding reference is to be added.
Juang, H.-M., and S.-Y. Hong, 2010: Forward semi-Lagrangian advection
with mass conservation and positive definiteness for falling
hydrometeors. *Mon.
Wea. Rev.*, *138*, 1778-1791.
…On Sat, Nov 6, 2021 at 9:41 AM gthompsnWRF ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In physics/module_mp_thompson.F90
<#770 (comment)>:
> @@ -3937,11 +3937,12 @@ subroutine mp_thompson (qv1d, qc1d, qi1d, qr1d, qs1d, qg1d, ni1d, &
do n = 1, niter
rr_tmp(:) = rr(:)
nr_tmp(:) = nr(:)
- call nislfv_rain_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1)
- call nislfv_rain_ppm(kte,dzq,vtnrk,nr,vtr,dtcfl,R2)
+ call nislfv_precip_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1)
What do you suggest using? Any idea?
I did give you my suggested name of subroutine in my comment. It is:
semi_lagrange_precip or perhaps better is semi_lagrange_sedim
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#770 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AVR3U6WNWUTPADXJODCZRNLUKVLE5ANCNFSM5HN4HQXA>
.
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>.
|
|
Your message came after I sent mine.
…On Sat, Nov 6, 2021 at 11:41 AM gthompsnWRF ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In physics/module_mp_thompson.F90
<#770 (comment)>:
> @@ -3937,11 +3937,12 @@ subroutine mp_thompson (qv1d, qc1d, qi1d, qr1d, qs1d, qg1d, ni1d, &
do n = 1, niter
rr_tmp(:) = rr(:)
nr_tmp(:) = nr(:)
- call nislfv_rain_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1)
- call nislfv_rain_ppm(kte,dzq,vtnrk,nr,vtr,dtcfl,R2)
+ call nislfv_precip_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1)
What do you suggest using? Any idea?
I did give you my suggested name of subroutine in my comment. It is:
semi_lagrange_precip or perhaps better is semi_lagrange_sedim
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#770 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKFOCLHZMQKEIP4TB6XEYPTUKVLE5ANCNFSM5HN4HQXA>
.
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>.
--
Ruiyu Sun, PhD
*IMSG* at NOAA/NWS/NCEP/EMC
5830 University Research Ct., Rm. 2097
College Park, MD 20740
***@***.*** ***@***.***>
301-683-3787
|
Adjustments were made. Thank you for your reviews and please let me know if you have any other suggestions. |
|
Ruiyu, Nicely done…. I am satisfied with the revised one. Songyou
… 2021. 11. 6. 오전 11:38, RuiyuSun ***@***.***> 작성:
The latter, semi_lagrange_sedim, is my preference. I wonder if my previous comments in git environment were delivered correctly. In addition to this naming, in 1st line of the description in the subroutine, ....for cloud is to be replaced with ....for falling hydrometeors. Also, the corresponding reference is to be added. Juang, H.-M., and S.-Y. Hong, 2010: Forward semi-Lagrangian advection with mass conservation and positive definiteness for falling hydrometeors. Mon. Wea. Rev., 138, 1778-1791.
… <x-msg://12/#>
On Sat, Nov 6, 2021 at 9:41 AM gthompsnWRF @.> wrote: @.* commented on this pull request. ------------------------------ In physics/module_mp_thompson.F90 <#770 (comment) <#770 (comment)>>: > @@ -3937,11 +3937,12 @@ subroutine mp_thompson (qv1d, qc1d, qi1d, qr1d, qs1d, qg1d, ni1d, & do n = 1, niter rr_tmp(:) = rr(:) nr_tmp(:) = nr(:) - call nislfv_rain_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1) - call nislfv_rain_ppm(kte,dzq,vtnrk,nr,vtr,dtcfl,R2) + call nislfv_precip_ppm(kte,dzq,vtrk,rr,rainsfc,dtcfl,R1) What do you suggest using? Any idea? I did give you my suggested name of subroutine in my comment. It is: semi_lagrange_precip or perhaps better is semi_lagrange_sedim — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#770 (comment) <#770 (comment)>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVR3U6WNWUTPADXJODCZRNLUKVLE5ANCNFSM5HN4HQXA <https://github.com/notifications/unsubscribe-auth/AVR3U6WNWUTPADXJODCZRNLUKVLE5ANCNFSM5HN4HQXA> . 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 <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 <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
Adjustments were made. Thank you for your reviews and please let me know if you have any other suggestions.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#770 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVR3U6XFM5FWIKXQJ7VNBI3UKVYZNANCNFSM5HN4HQXA>.
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>.
|
gthompsnWRF
left a comment
gthompsnWRF
left a comment
There was a problem hiding this comment.
Thanks Ruiyu for making the small changes suggested!
|
Sure! Thank you all for the careful reviews.
…On Mon, Nov 8, 2021 at 11:15 AM gthompsnWRF ***@***.***> wrote:
***@***.**** approved this pull request.
Thanks Ruiyu for making the small changes suggested!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#770 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKFOCLCUF66MWGP6MG5N3QLUK7ZTBANCNFSM5HN4HQXA>
.
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>.
--
Ruiyu Sun, PhD
*IMSG* at NOAA/NWS/NCEP/EMC
5830 University Research Ct., Rm. 2097
College Park, MD 20740
***@***.*** ***@***.***>
301-683-3787
|
|
Two additional suggestions were made by Songyou: |
mzhangw
left a comment
mzhangw
left a comment
There was a problem hiding this comment.
minor format changes need to be made.
climbfuji
left a comment
climbfuji
left a comment
There was a problem hiding this comment.
I suggest to revert the whitespace changes that I mentioned in a follow-up PR.
5 participants
This PR is to add semi-Lagrangian sedimentation of graupel into the Thompson microphysics scheme.