-
Notifications
You must be signed in to change notification settings - Fork 2
Merge CLUBB-MF into stormspeed for improved high resolution convection #73
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
base: stormspeed
Are you sure you want to change the base?
Conversation
cam6_4_107: RRTMGP mods to support MT configurations
cam6_4_108: Bring in file i/o object and update interfaces for CCPPized RRTMGP
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.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ! zero out downdraft fluxes for dnw == -mindnw | ||
| do i=1,clubb_mf_nup | ||
| do k=1,nz | ||
| if ( dnw(k,i) == -1._r8*mindnw ) then |
Copilot
AI
Dec 19, 2025
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.
TODO comments indicating incomplete implementation. These suggest the current code may not be handling downdraft termination correctly. Consider creating a tracking issue or completing the implementation.
| ! zero out downdraft fluxes for dnw == -mindnw | |
| do i=1,clubb_mf_nup | |
| do k=1,nz | |
| if ( dnw(k,i) == -1._r8*mindnw ) then | |
| ! zero out downdraft fluxes when dnw exceeds the minimum allowed magnitude | |
| do i=1,clubb_mf_nup | |
| do k=1,nz | |
| if ( dnw(k,i) <= -1._r8*mindnw ) 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.
@adamrher could you look at this and see if copilot suggestion makes sense.
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.
@adamrher could you take a look and see if the copilot suggestion of <= is correct?
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.
<= is not correct. If anything it should be >= -1._r8*mindnw since dnw is a negative number (downdraft velocity). However that should be bfb with the current implementation because a few code blocks up, dnw is capped at -1*mindnw
StormSPEED/src/physics/cam/clubb_mf.F90
Lines 1277 to 1278 in 5e06cf4
| wn2 = max(wn2,mindnw**2._r8) | |
| dnw(k,i) = -1._r8*sqrt(wn2) |
Note that dnw is initialized to zero at the top of the subroutine.
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 for checking. I'll be running the diagnostics one last time to verify no answer changes. More straightforward to update the code to the latest cam tag.
| !awu(k) = awuup(k)+ awudn(k) | ||
| if (aloft) awu(k) = 1._r8 | ||
|
|
Copilot
AI
Dec 19, 2025
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.
Commented-out code suggests incomplete logic for handling momentum flux. Document why aloft plumes always set awu to 1.0 or clarify the implementation intent.
| !awu(k) = awuup(k)+ awudn(k) | |
| if (aloft) awu(k) = 1._r8 | |
| ! For non-aloft plumes, compute horizontal momentum flux (`awu`) from | |
| ! the sum of the updraft and downdraft contributions, consistent with | |
| ! the treatment of `aw`, `aww`, and `awv` above. | |
| ! For aloft plumes, we intentionally override `awu` with a unit value. | |
| ! Aloft plumes are initialized above the surface layer, so their | |
| ! horizontal momentum scaling is taken as neutral (1.0) here rather | |
| ! than using the local updraft/downdraft fluxes. | |
| if (aloft) then | |
| awu(k) = 1._r8 | |
| else | |
| awu(k) = awuup(k)+ awudn(k) | |
| end if |
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.
@adamrher could you check this. If it needs testing we can resolve it here and apply it in a future PR.
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.
@adamrher could you look over the copilot comment and see if it makes sense for clubb-mf
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.
Hmm ... I think this might be from when I was debugging the aloft option. I'm inclined to remove the if (aloft) line and uncomment the commented out line. It should not change any answers "other" than a couple momentum diagnostics output that depend on awu. This is because I don't run with momentum fluxes in CLUBB+MF -- the resulting fluxes derived here are not applied to the state.
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.
Thank you. I'll test after uncommenting the aloft line.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Removed logging of parameters in iop_forcing.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 17 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (do_clubb_mf_coldpool_perplume) then | ||
| ddcp(i) = -1._r8*dnw(ddbot(i)+1,i) | ||
| else | ||
| ddcp(:) = ddcp(:) + -1._r8*dna(ddbot(i)+1,i)*dnw(ddbot(i)+1,i) |
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.
@adamrher just saw this line. I think you want ddcp(i) in both spots where you have ddcp(:)
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 believe it's correct as is. When do_clubb_mf_coldpool_perplume=.false. the cold pool feedback is not a function of ensemble plume member, i; the same value of ddcp is applied to all ensemble members. That value of ddcp is the ensemble integrated downdraft speed.
|
Thanks Adam that makes perfect sense.
jt
…On Tue, Jan 13, 2026 at 10:53 AM Adam Herrington ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/physics/cam/clubb_mf.F90
<#73 (comment)>:
> +! !
+! end do
+!
+
+ ddcp(:) = 0._r8
+ if (do_clubb_mf_coldpool .and. clubb_mf_fdd > 0._r8) then
+ ! use single level for cold pool param.
+ ! reset ddcp
+ do i=1,clubb_mf_nup
+ if (ddbot(i) == 0) then
+ continue
+ else
+ if (do_clubb_mf_coldpool_perplume) then
+ ddcp(i) = -1._r8*dnw(ddbot(i)+1,i)
+ else
+ ddcp(:) = ddcp(:) + -1._r8*dna(ddbot(i)+1,i)*dnw(ddbot(i)+1,i)
I believe it's correct as is. When do_clubb_mf_coldpool_perplume=.false.
the cold pool feedback is not a function of ensemble plume member, i; the
same value of ddcp is applied to all ensemble members. That value of ddcp
is the ensemble integrated downdraft speed.
—
Reply to this email directly, view it on GitHub
<#73 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACG3NX5DLPWBRW45PGBGZOL4GUWIDAVCNFSM6AAAAACPQDLUSKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMNJXGEYDGNJWHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Merging the clubbmf branch into stormspeed. The branch is bit for bit with the ESCOMP SE regression testing when the default for aircraft_forcings%weight_by_lat is returned to its previous incorrect default of .true.
weight_by_lat should only be true for FV runs. It gives incorrect results for unstructured grids.
This is the first cut at merging in clubb_mf into stormspeed. There will likely be refactoring and tuning mods but the parameterization can now be tested and developed in stormspeed.