Complete CCPPization of gravity wave drag parameterizations (CAM4/5/6/7) #292
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
initial gw mods commit for update to latest debugging cam run of ccpp gw quick adds of diagnostic templates for gw get rid of an old unused directory
…ecific scheme code
Co-authored-by: Jesse Nusbaumer <[email protected]>
Co-authored-by: Jesse Nusbaumer <[email protected]>
nusbaume
approved these changes
Oct 20, 2025
Collaborator
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Thanks for resolving most of my concerns @jimmielin! There are a couple additional requests, and a few question that we probably need scientist input on, but nothing that should need a re-review from me. Thanks agian!
| dimensions = (horizontal_loop_extent, vertical_interface_dimension) | ||
| intent = in | ||
| [ rhoi ] | ||
| standard_name = air_density_at_interfaces_for_gravity_wave_drag |
Collaborator
There was a problem hiding this comment.
I was assuming that it was the dry air density because the gas constant (rair) used here is for dry air. So if it is using the moist pressure but the dry air gas constant then my guess is that the equation is just not completely physically consistent in it's current form (i.e. it's a small "science" bug). Maybe it is worth asking the scientists what they think?
Comment on lines
1809
to
1810
| !kwvrdg = 0.001_kind_phys/(100._kind_phys) | ||
| kwvrdg = 1e-5_kind_phys |
Collaborator
There was a problem hiding this comment.
Remove commented-out code?
|
|
||
| implicit none | ||
| private | ||
| save |
Collaborator
There was a problem hiding this comment.
Thanks all for the discussion! I might still lean towards getting rid of the save line. This is partly because this code is frequently examined by scientists, so the less non-necessary lines we have the better. Plus if one is doing something where it is "needed" then my guess is that there is a different issue (e.g. a variable being out-of-scope) that is being masked. Finally there are plenty of modules in CAM that don't have the save attribute but do have module-level variables that are assumed to be kept in memory, so if there was really a compiler issue I think we would have run into it in CESM by now.
| [ccpp-table-properties] | ||
| name = gravity_wave_drag_common | ||
| type = scheme | ||
| dependencies = ../../to_be_ccppized/linear_1d_operators.F90,../../to_be_ccppized/vdiff_lu_solver.F90,gw_diffusion.F90 |
Collaborator
There was a problem hiding this comment.
Ahh, sorry, I forgot that interpolate_data was a CAM-SIMA utility! In that case I would not include it in the list of dependencies. Instead it is something we'll just need to tackle later if/when we want this scheme to truly be portable.
peverwhee
requested changes
Oct 27, 2025
Collaborator
peverwhee
left a comment
peverwhee
left a comment
There was a problem hiding this comment.
a few things, nothing major! thanks @jimmielin !
schemes/gravity_wave_drag/gravity_wave_drag_moving_mountain.F90
Outdated
Show resolved
Hide resolved
schemes/gravity_wave_drag/gravity_wave_drag_moving_mountain.F90
Outdated
Show resolved
Hide resolved
Co-authored-by: Courtney Peverley <[email protected]>
Co-authored-by: Courtney Peverley <[email protected]>
Co-authored-by: Courtney Peverley <[email protected]>
Co-authored-by: Courtney Peverley <[email protected]>
Co-authored-by: Courtney Peverley <[email protected]>
peverwhee
approved these changes
Oct 29, 2025
Member
Author
|
preliminary run of regression tests based on cam6_4_132 passed (subject to NLFAIL and existing failures) - the |
jimmielin
added a commit
that referenced
this pull request
Nov 28, 2025
…erizations (#336) Tag name (The PR title should also include the tag name): atmos_phys0_19_000 Originator(s): @jimmielin @jtruesdal List all `development` PR numbers included in this PR and the title of each: - Complete CCPPization of gravity wave drag parameterizations (CAM4/5/6/7) - #292 List all automated tests that failed, as well as an explanation for why they weren't fixed: NLCOMP failures in CAM-SIMA due to new gw_drag_input_nl NLCOMP failures in CAM due to removal of unused namelist param
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Originator(s): @jimmielin @jtruesdal
Description (include issue title and the keyword ['closes', 'fixes', 'resolves'] and issue number):
gw_common.F90schemegravity_wave_drag_common), and necessary interstitials for preparation of vertical profiles (gravity_wave_drag_prepare_profiles) and vertical tapering (gravity_wave_drag_top_taper) near top of model;gravity_wave_drag_orographicand test suitesuite_gw_cam4.xml-- this is tested to be bit-for-bit in CAM-SIMA.This PR has overlapping changes with the HB diff atmos_phys in terms of coords1d.F90 and moving of some diffusion solver files. There is no particular order in which these two would have to be merged since the changes are replicated in both branches. This is just a note in case some merge conflicts show up, which should just be trivial like whitespace.
List all namelist files that were added or changed:
List all files eliminated and why: N/A
List all files added and what they do:
List all existing files that have been modified, and describe the changes:
(Helpful git command:
git diff --name-status development...<your_branch_name>)List all automated tests that failed, as well as an explanation for why they weren't fixed: N/A
Is this an answer-changing PR? If so, is it a new physics package, algorithm change, tuning change, etc?
New physics package.
B4B with existing CAM.
If yes to the above question, describe how this code was validated with the new/modified features:
CAM-SIMA tested with snapshots from FHIST_C4 for the
suite_gw_cam4.xmltesting SDF and snapshot from FHISTC_LTso for thesuite_gw_cam7_se.xmltesting SDF.Changes to CAM are b4b