Skip to content

Refactor C3P and introduce CCPP interface with MPAS compatibility; migrate to submodule#373

Merged
rhaesung merged 21 commits into
ufs-community:ufs/devfrom
lisa-bengtsson:c3
Jun 10, 2026
Merged

Refactor C3P and introduce CCPP interface with MPAS compatibility; migrate to submodule#373
rhaesung merged 21 commits into
ufs-community:ufs/devfrom
lisa-bengtsson:c3

Conversation

@lisa-bengtsson

@lisa-bengtsson lisa-bengtsson commented May 4, 2026

Copy link
Copy Markdown
Collaborator

This PR introduces a major refactoring of the community convective cloud parameterization (C3P), including a new CCPP-facing interface that is compatible with shared MPAS code. As part of this effort, the codebase is migrated from a subdirectory into a standalone repository located here: ufs-community/c3#2

  • Kept the existing CCPP-facing interface from ccpp-physics/physics/CONV/C3/cu_c3_driver.F90 mostly intact, with corresponding updates to newly added cu_c3_driver.meta.
  • Extracted the MPAS-facing driver from module_cu_c3.F90 and renamed it cu_c3_driver_mpas.F90.
  • Added the needed module_cu_c3 dependencies to the CCPP metadata/build path.
  • Temporarily call initModConvParGF() to initialize default namelist/configuration variables. Longer term, these should likely be exposed through the standard UFS/CCPP namelist mechanism.
  • Import only the plume-control variables needed by the CCPP driver loop; the remaining initialized module variables are used internally by CUP_C3.
  • Reworked the CCPP driver to transpose CCPP (i,k) arrays into the shared C3 (k,i) layout before calling CUP_C3.
  • Reconstructed MPAS-style thermodynamic states in the CCPP driver, including temp_old, temp_new, temp_new_BL, and temp_new_ADV, using dynamics and PBL tendencies where available.
  • Added wiring to output PBL t and q tendencies from GFS_PBL_generic_post.F90 and mynnedmf_wrapper_post.F90
  • Converted CCPP specific humidity inputs/tendencies to dry-air mixing ratio for consistency with C3.
  • Adjusted land/sea-mask handling, surface flux units, surface temperature, LCL/source-parcel perturbations, and related preprocessing to more closely match the MPAS driver.
  • Replaced the legacy shallow/deep CCPP call block with a direct CUP_C3 path patterned after the MPAS driver and the shared C3 call structure.
  • Kept the existing CCPP postprocessing section, adding mappings from the shared C3 plume-indexed outputs back into the legacy CCPP arrays.
  • Preserved the existing OpenACC directives in the CCPP driver where possible, but CUP_C3 itself will need OpenACC/device treatment before this is GPU-ready.
  • Renamed relevant C3 source files from .F to .F90 so they compile consistently as free-form Fortran.

@dustinswales dustinswales left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lisa-bengtsson This all looks good to me. I think we can delete mynnedmf_wrapper_post ant use GFS_DCNV_generic_post everywhere?
I had some suggestions in the C3 submodule PR that will require changes here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grantfirl I think this file can be removed entirely and replaced with GFS_PBL_generic_post. Right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a subtlety that the scheme wants pbl_tke, and I am a bit uncertain how to make it generic.

I see in the wrapper that there is:
[qke]
standard_name = nonadvected_turbulent_kinetic_energy_multiplied_by_2
long_name = 2 x tke at mass points
units = m2 s-2
dimensions = (horizontal_loop_extent,vertical_layer_dimension)
type = real
kind = kind_phys
intent = inout
[qke_adv]
standard_name = turbulent_kinetic_energy
long_name = turbulent kinetic energy
units = J kg-1
dimensions = (horizontal_loop_extent,vertical_layer_dimension)
type = real
kind = kind_phys
intent = inout

GFS_PBL_generic_post has "turbulent_kintetic_energy" and it is a tracer that is advected. After talking to @joeolson42 it seems that this is not always used by default and then set to 0.

From Joe: "qke_adv is the fully advected TKE, when tke_advect = T, otherwise = 0.
Basically, it's a scalar that is advected when the TKE advection flag is activated, otherwise that array is neglected.

QKE is always allocated and set equal to QKE_ADV when TKE advection is activated, so we only have to output QKE and then you'll get a prognosed variable that is either advected or not."

The problem is that I then, depending on scheme, have to divide by 2 inside the C3 driver to arrive at "pbl_tke".

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dustinswales Most of GFS_pbl_generic_post.F90 isn't needed when MYNN PBL is being used. For example all other PBL schemes use the vertically-diffused tracer subset array, whereas MYNN does not. The only thing the MYNN PBL needs is to handled tendencies afterwards, hence the simple scheme. Alternatively, one could use GFS_pbl_generic_post.F90 and just check for MYNN to bypass most of that code.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grantfirl @lisa-bengtsson @joeolson42 Everything in mynn_wrapper_post.F90 is repeated in GFS_PBL_generic_post.F90. So why have both?
We could configure GFS_PBL_generic_post.F90 to only do what we want it to for MYNN (i.e., accumulate tends), and skip the rest.
One way to do this is to move the move the tendency application piece to the top of GFS_PBL_generic_post and return afterwards when using mynnedmf?
Also, organizationally it makes sense for the state/tendency update to occur in a Host interstitial, not a Scheme interstitial.

@lisa-bengtsson Sorry for cluttering your C3 PR with other housekeeping issues. If we agree, I will open an issue and address this later in a follow up PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why have both?

Ya, not technically necessary, but I think in the past, Joe has kinda preferred to keep MYNN using different interstitials (e.g. sgscloud) that are simpler/streamlined than the generic versions.

It's also not possible to move the tendency application first in that file due to the translation between the vertically-diffused tracer array logic. But, it doesn't really matter. It's easy to put a check for MYNN and skip everything except the tendency application.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dustinswales @grantfirl yes, opening a separate issue/PR for this would be better I think. From my point of view creating a GFS_generic_physics_post (or similar) to accompany the physics_pre that you created would be great for extracting the dynamics tendencies that are now used in multiple scheme and re-computed depending on suite. For instance, for saSAS I added that in GFS_micro_pre/post a while ago, that could be moved. This "overarching" pre/post could also include the general thermodynamic states that multiple schemes use as we discussed (such as moist air density...). Then having one "turbulent_kinetic_energy" regardless of scheme, and one place for the PBL tendencies regardless of scheme would also be fantastic. We could remove the pre-fix "GFS" if it is unclear that is is more generic than that.

Comment thread .gitmodules Outdated
[submodule "physics/CONV/C3"]
path = physics/CONV/C3
url = https://github.com/lisa-bengtsson/c3
branch = c3

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lisa-bengtsson Will the c3 branch always be the branch that the CCPP repo points to? I noticed that there were both main and c3 branches in the C3 repo. Some submodule schemes use main. Others use a separate branch like is done here. Just wanting to confirm your intent. Otherwise, if you do intend to use main, there should probably be a PR in the C3 submodule from the c3 branch to main to go along with this PR set.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grantfirl yes - that is this one: ufs-community/c3#2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks.

@grantfirl

Copy link
Copy Markdown
Collaborator

@lisa-bengtsson If I wanted to review changes to the C3 code represented by this PR (since it contains both changes to the scheme and changing it to a submodule simultaneously), do you know if the start of the submodule C3 repo was identical to the version of C3 in the CCPP? I.e. if I were to look at differences between the commit that you're using in this PR versus the initial C3 commit, would that show me what has changed within the actual scheme internals?

@lisa-bengtsson

Copy link
Copy Markdown
Collaborator Author

No, unfortunately not - more or less everything is new. I would treat it as if this was a new scheme that entered CCPP and maybe review it in terms of CCPP compliance?

Comment thread physics/Interstitials/UFS_SCM_NEPTUNE/GFS_PBL_generic_post.F90 Outdated
Comment thread physics/PBL/MYNN_EDMF/mynnedmf_wrapper_post.F90

@grantfirl grantfirl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The part that remains in the CCPP repo looks great to me. Just minor comment to improve robustness, but not necessary to change if you don't want to.

@lisa-bengtsson

Copy link
Copy Markdown
Collaborator Author

The part that remains in the CCPP repo looks great to me. Just minor comment to improve robustness, but not necessary to change if you don't want to.

Thank you Grant, I will address these this afternoon.

@grantfirl

Copy link
Copy Markdown
Collaborator

@lisa-bengtsson Could you mention ufs-community/c3#2 in the PR description please?

@rhaesung rhaesung left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I left a few minor comments on ufs-community/c3#2.

@gspetro-NOAA

Copy link
Copy Markdown

@dustinswales Has your requested change been addressed? It sounds like perhaps a separate issue was going to be created?

@grantfirl grantfirl dismissed dustinswales’s stale review June 1, 2026 14:40

The requested changes will be in a followup PR.

@dustinswales dustinswales self-requested a review June 3, 2026 17:25
@gspetro-NOAA

Copy link
Copy Markdown

The C3 sub-PR was merged, and Lisa updated the hash and .gitmodules, so this PR should be set to merge.

@rhaesung rhaesung merged commit f922ca6 into ufs-community:ufs/dev Jun 10, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants