-
Notifications
You must be signed in to change notification settings - Fork 153
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
cam6_4_080: Derive new geopotential fields whenever water species are updated (not just water vapor) #1288
cam6_4_080: Derive new geopotential fields whenever water species are updated (not just water vapor) #1288
Conversation
…t just water vapor
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.
Looks good (just a little suggestion but either way is fine with me) thanks!
derive_new_geopotential = .true. | ||
exit const_water_loop | ||
endif | ||
endif |
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 you could also do:
const_water_loop: do m = dry_air_species_num + 1, thermodynamic_active_species_num
if(ptend%lq(m)) then
! is water species?
derive_new_geopotential = .true.
exit const_water_loop
end if
end do
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 @PeterHjortLauritzen, that makes more sense, updated!
I will update with the regression tests results when they're available to see which compsets need a closer science check...
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 looks like I had to change
if(ptend%lq(m)) then
to
if(ptend%lq(thermodynamic_active_species_idx(m))) then
as otherwise the m
indexing is not correctly translated to the constituent indices. I got a CAM6 test failure when finalizing the regression tests and realized that the indexing had to be fixed. I am rerunning the tests now.
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.
good catch!
I ran the Derecho Intel and Izumi NAG regression tests to check which compsets are affected by this change. This should only change answers if physics schemes accumulate tendencies, depend on geopotential height, and have intermediate tendencies that do not update water vapor but update other water species. The good news is that it seems like only the CAM4 physics are affected because of the issue impacting RK (detrainment of convective condensate into cloud liquid water has a tendency update and the following cldwat call uses Izumi nag shows only CAM4 physics
Derecho Intel shows CAM4 (QPX2000) and the others are expected failures:
|
Merge pull request ESCOMP#1288 from jimmielin/hplin/geopotential_t_water_species
Fixes #1286.