Conversation
…rations on several variables with latest clubb interface and externals.
There was a problem hiding this comment.
Pull request overview
This PR merges updated CLUBB+MF (clubb-mf) changes into CAM, extending the CLUBB interface to support the enhanced integrate_mf call, adding new EDMF diagnostics/outputs, and introducing additional namelist controls for CLUBB+MF behavior (radiation coupling, microphysics, TKE contribution, etc.).
Changes:
- Expanded
clubb_intr.F90CLUBB+MF interface: new pbuf fields, history coords, diagnostic outputs, and enhanced MF plumbing. - Added new CLUBB+MF namelist entries + defaults, and wired them into
build-namelist. - Updated COSP namelist documentation text (but introduced encoding issues that need correction).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 18 comments.
| File | Description |
|---|---|
src/physics/cam/clubb_intr.F90 |
Adds/updates CLUBB+MF fields, diagnostics, and MF integration interface wiring. |
bld/namelist_files/namelist_definition.xml |
Adds new CLUBB+MF namelist definitions and edits COSP localtime documentation. |
bld/namelist_files/namelist_defaults_cam.xml |
Adds default values for the new CLUBB+MF namelist entries. |
bld/build-namelist |
Adds add_default(...) plumbing for (most) new CLUBB+MF namelist variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
adamrher
left a comment
adamrher
left a comment
There was a problem hiding this comment.
Approving this provided @jtruesdal merges my PR to this PR branch.
modifications to resolve legacy mass conservation issues
cacraigucar
left a comment
cacraigucar
left a comment
There was a problem hiding this comment.
I am submitting this. I still have clubb_mf.F90 to review, but that will need to be next week.
| #endif | ||
|
|
||
| ! CFL limiter vars | ||
| real(r8), parameter :: cflval = 1._r8 |
There was a problem hiding this comment.
Since this is a parameter constant, it would be good to have a comment on what it is
There was a problem hiding this comment.
"Upper limit on Courant number for stability"
| @@ -2726,6 +2991,33 @@ subroutine clubb_tend_cam( state, ptend_all, pbuf, hdtime, & | |||
| ! Initialize err_info with parallelization and geographical info | |||
| call init_err_info_api(ncol, lchnk, iam, state_loc%lat*rad2deg, state_loc%lon*rad2deg, err_info) | |||
|
|
|||
|
|
|||
| if (do_clubb_mf) then | |||
| ! SVP | |||
There was a problem hiding this comment.
What does this comment mean? If it is just someone's initials, please replace with a comment on what this block is doing
There was a problem hiding this comment.
This has since been moved into clubb_mf.F90, and the SVP comment was removed (which referred to saturation vapor pressure; the abbreviation is used elsewhere in comments in our code).
There was a problem hiding this comment.
New comment with words not initials.
hook up clubb-mf to deep infrastructure
address git copilot review
…rag fix for clubb_mf deep.
cacraigucar
left a comment
cacraigucar
left a comment
There was a problem hiding this comment.
I stopped reviewing at line 2709 in clubb_mf.F90 as the routines from this point to the end of the file appear to be copies which are better addressed with the new issue of opening a shared library location in ESCOMP/atmopsheric_physics for these routines. The code from line 2709 to the end of file is not in shape that it should reside in ESCOMP/CAM long-term. There is one possible copyright issue which does need to be addressed before this is committed in that section of the code.
| subroutine knuth(kiss_gen,lambda,kout) | ||
| !********************************************************************** | ||
| ! Discrete random poisson from Knuth | ||
| ! The Art of Computer Programming, v2, 137-138 | ||
| ! By Adam Herrington | ||
| !********************************************************************** |
There was a problem hiding this comment.
@briandobbins - As we discussed, is there an open source version of this available?
| subroutine hormann(kiss_gen,lambda,kout) | ||
| !********************************************************************** | ||
| ! Discrete random poisson | ||
| ! Implements Poisson Transformed Rejection with Squeeze (PTRS) | ||
| ! from W. Hormann Insurance: Mathematics and Economics 12, 39-45 (1993) | ||
| ! By Jake Reschke | ||
| !********************************************************************** |
There was a problem hiding this comment.
@briandobbins - Here is the other routine to find an open source version of
| use shr_kind_mod, only: r8=>shr_kind_r8 | ||
| use spmd_utils, only: masterproc | ||
| use cam_logfile, only: iulog | ||
| use cam_abortutils,only: endrun |
There was a problem hiding this comment.
I asked Claude to summarize the shared routines I was seeing in this module. For the sake of this PR, let's open an issue to address this at a later date in ESCOMP/atmospheric_physics
Summary
Duplicated / adapted helper routines in clubb_mf.F90
Several helpers in clubb_mf.F90 are copied or adapted from existing code in the tree — uwshcu.F90 and the ZM scheme
src/atmos_phys/schemes/zhang_mcfarlane/zm_convr.F90.
Routine (clubb_mf.F90) |
Source | Relationship |
|---|---|---|
roots (L2495) |
uwshcu.F90:4728 |
Verbatim copy (cosmetic diffs only) |
qsat_hPa (L3465) |
zm_convr.F90:2612 |
Verbatim copy (only kind_phys→r8) |
entropy (L3336) |
zm_convr.F90:1467 |
Copy, constants renamed to physconst |
ientropy (L3359) |
zm_convr.F90:1495 |
Copy of Brent-solver core; error handling swapped |
buoyan_dilute (L2709) |
zm_convr.F90:778 |
Heavily adapted (not verbatim) |
parcel_dilute (L2964) |
zm_convr.F90:1130 |
Heavily adapted (not verbatim) |
enthalpy (L3483) |
(none in ZM) | New, cloned from entropy |
ienthalpy (L3502) |
(none in ZM) | New, self-admitted clone of ientropy |
Notes
qsat_hPa— byte-identical to ZM's version except the kind suffix (sameqsat_watercall, samep*100/es*0.01
conversions).roots— near-verbatim copy of theuwshcuversion; only whitespace/comment differences.entropy/ientropy— same algorithm as ZM. ZM passescpliq,cpwv,rh2oas args (CCPP) and uses ZM constant names (rl,
tfreez,eps1,cpres,rgas); this copy usesphysconst.ientropy's Brent loop is line-for-line identical; only the
non-convergence path differs (ZM setserrmsg/errflg, this callsendrun).buoyan_dilute/parcel_dilute— same dilute-CAPE algorithm, reworked from multi-column (ncol) to the single-column,
nup-plume ensemble, made grid-orientation-agnostic, plus thetht_tweaksenthalpy path. Genuinely diverged — not a drop-in
duplicate.ienthalpy— per its own comment, "identical with iENTROPY, only function calls swapped": a second copy of the Brent
solver within the same file.enthalpyis likewise a variant ofentropy.
Suggested follow-ups (ranked)
qsat_hPa— cleanest win; identical in two files. Hoist to a shared module anduseit in both.ientropy/ienthalpy— collapse the two in-file Brent solvers into one (procedure arg or entropy-vs-enthalpy flag).entropy/ientropyvs ZM — shareable in principle, but CCPP-vs-endrunand constant-passing differences make it
non-trivial; lower priority.
buoyan_dilute / parcel_dilute are diverged enough that keeping them independent is reasonable.
There was a problem hiding this comment.
@cacraigucar @jtruesdal I think I'd rather just remove all this duplicate ZM code entirely. These are used for the Lopt = 4,5 options, which I've never gotten to work properly. I can also just experiment with it on my branch.
There was a problem hiding this comment.
@adamrher just to double check, remove lopt = 4,5 as possible options in the released code as well as
qsat_hPa, entropy, ientropy buoyan_dilute, parcel_dilute, enthalpy, ienthalpy routines?
| ! Invert the entropy equation -- use Brent's method | ||
| ! Brent, R. P. Ch. 3-4 in Algorithms for Minimization Without Derivatives. Englewood Cliffs, NJ: Prentice-Hall, 1973. |
There was a problem hiding this comment.
@briandobbins - Here is another routine to see if an open source version exists
…o .gitmodules for updated interp_mod interface, will replace with new official tag once its passes regression testing.
jtruesdal
changed the title
| url = https://github.com/jtruesdal/CAM_FV3_interface.git | ||
| fxrequired = AlwaysRequired | ||
| fxtag = fv3int_061924 | ||
| fxDONOTUSEurl = https://github.com/ESCOMP/CAM_FV3_interface.git | ||
| fxtag = fv3int_092126 | ||
| fxDONOTUSEurl = https://github.com/jtruesdal/CAM_FV3_interface.git |
There was a problem hiding this comment.
This needs to become an official tag in the ESCOMP repo
There was a problem hiding this comment.
Thanks for the reminder. I will get this very minor change tagged in fv3 and include the official tag here. I just wanted to test it thoroughly first.
| close(iunit) | ||
| end if | ||
|
|
||
| if (clubb_mf_Lopt < 0 .or. clubb_mf_Lopt > 8 ) & |
There was a problem hiding this comment.
If option 4 and 5 get deleted, you will need to check that they are not set here
cacraigucar
left a comment
cacraigucar
left a comment
There was a problem hiding this comment.
Resolved a number of conversations (and added a few more comments)
| ztopm1(:) = zm(ksfcm) | ||
| return | ||
|
|
||
| else ! we have positive surface buoyancy and moisture flux lets do mass-flux |
There was a problem hiding this comment.
Can eliminate this else (and the end if at the end of this routine). I don't have a strong preference that you need to shift the code over space wise if you do this.
There was a problem hiding this comment.
Thanks for resolving that makes it easier to see whats left.
I left the spacing so we don't have that show up in our diffs. I also changed the missed misspelling and made the logicals into parameters.
…up, new parameters for magic numbers, fix for out of bounds array access.
cacraigucar
left a comment
cacraigucar
left a comment
There was a problem hiding this comment.
resolved the changes that were made in the recently committed code
This PR merges updated CLUBB+MF (clubb-mf) changes into CAM, extending the CLUBB interface to support the enhanced integrate_mf call, adding new EDMF diagnostics/outputs, and introducing additional namelist controls for CLUBB+MF behavior (radiation coupling, microphysics, TKE contribution, etc.).
Changes:
The majority of the code updates reside in clubb_mf.F90. In addition to the new science, clubb_mf is now agnostic to the direction of the vertical level, adjusting the looping and indexes as needed depending on the top down or bottom up nature of the input arrays. Additionally the number of vertical levels for the new mf arrays are consistent with their associated thermodynamic or momentum grid. There are order of operation differences between this update and Adam's original cam6_4_124 version of clubb_mf which cause roundoff precision differences from the base code. On top of these roundoff differences there are bug fixes which are answer changing.
This PR closes #1370