Skip to content
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

Remove CAM dependencies and add CCPP layer for PUMAS v1 routines #71

Open
wants to merge 16 commits into
base: main_cam
Choose a base branch
from

Conversation

nusbaume
Copy link

This PR removes all explicit uses of CAM and/or CESM share modules, and instead adds internal PUMAS routines to accomplish the same goals. The one exception is for calls to wv_sat_methods which will need to remain a dependency for now (but can likely be CCPP-ized as well later). Part of this effort included removing the no-longer-used micro_pumas_data.F90 file.

This PR also adds a CCPP Fortran module and metadata file to enable CCPP-compliant calls to micro_pumas_init and micro_pumas_tend, as well as a stub metadata file for the proc_rates_type DDT, which will need to be visible to the CCPP-framework in order for it to be passed to external CCPP schemes (e.g. diagnostics).

Testing:

Successfully passed all aux_cam tests on Derecho with the Intel compiler using cam6_4_055 baselines. Also passed all aux_pumas tests except for the one test failure described in #70

@nusbaume nusbaume self-assigned this Jan 16, 2025
@nusbaume nusbaume requested a review from Katetc January 16, 2025 21:24
Copy link
Collaborator

@Katetc Katetc left a comment

Choose a reason for hiding this comment

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

Phew! I did it. I didn't see much that was actually an issue except the units on DCS. Thanks Jesse!

@@ -1,4 +1,7 @@
====================================
.. pumas_cam-release_v1.38
January 15, 2025 - Removed all CAM dependencies from core PUMAS routines except wv_sat_methods. Also added a Common Community Physics Package (CCPP) wrapper for micro_pumas_v1.F90, including two new CCPP metadata files. A new CAM tag will be made which calls the wrapper instead of PUMAS directly, although this isn't technically required.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is kind of silly, but could you add the name of the wrapper file (micro_pumas_ccpp.F90) and possibly the new calling order for CAM-SIMA in here?

@@ -147,8 +149,6 @@ subroutine proc_rates_allocate(this, psetcols, nlev, ncd, warm_rain, errstring)
! Routine to allocate the elements of the proc_rates DDT
!--------------------------------------------------------------

use cam_abortutils, only: endrun

Copy link
Collaborator

Choose a reason for hiding this comment

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

The fact that this is just here and never used makes me giggle. It's a clean up that was needed for all end uses.

#process rates DDT to allow it
#to be passed between PUMAS and
#any CCPP intersitial schemes.
#################################
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a note that this doesn't work yet. Or a Todo comment so we can track the todos.

@@ -1,6 +1,6 @@
module tau_neural_net_quantile

use shr_kind_mod, only: r8=>shr_kind_r8
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, this type is changed, but it's not used below. There are a few literals in this file where the typing is not specified. It would be really nice if you could add "_r8" to the real literals in this file and the typing for the integers, too, possibly?

@@ -86,8 +88,6 @@ module micro_pumas_utils
avg_diameter_vec

! 8 byte real and integer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this comment too

[micro_accre_enhan_in]
standard_name = microphysics_accretion_enhancement_factor
long_name = microphysics accretion enhancement factor
units = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

None?

intent = in
[micro_pdel_in]
standard_name = microphysics_air_pressure_thickness
long_name = microphysics air pressure thickness
Copy link
Collaborator

Choose a reason for hiding this comment

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

"microphysics air pressure layer thickness"?

[micro_qsatfac_in]
standard_name = microphysics_subgrid_cloud_water_saturation_scaling_factor
long_name = microphysics subgrid cloud water saturation scaling factor
units = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another factor - units none?

intent = out
[micro_prect_out]
standard_name = microphysics_lwe_large_scale_precipitation_rate_at_surface
long_name = microphysics LWE large scale precipitation rate at surface
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, is LWE liquid water equivalent? I can see that being used here because this total precip rate includes rain and snow, but I feel like total should be in the long name and LWE is confusing. We actually don't use the LWE acronym in PUMAS at all.

[micro_lamcrad_out]
standard_name = microphysics_cloud_particle_size_distribution_slope_parameter
long_name = microphysics cloud particle size distribution slope (lambda) parameter
units = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm ok with these being "1" but I've written so many comments now that it feels disingenuous.

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.

4 participants