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

Add CAM6 Suite Definition File #7

Merged
merged 11 commits into from
Mar 27, 2020
Merged

Conversation

nusbaume
Copy link
Collaborator

@nusbaume nusbaume commented Feb 18, 2020

First pass at creating a CAM6 Suite Definition File (SDF).

This is the same as PR #4, which had to be closed due to a broken fork caused by permission changes to the NCAR/atmospheric_physics repo. The issues found in the original PR have (hopefully) been fixed, so this PR is ready for review.

Tests run:

xmllint --noout --schema gold2718/ccpp-framework/schema/suite_v1_0.xsd suite_cam6.xml

XML file validates except for the partition tags, which currently do not exist in the CCPP framework physics suite schema.

@nusbaume nusbaume added the enhancement New feature or request label Feb 18, 2020
@nusbaume nusbaume self-assigned this Feb 18, 2020
Copy link
Collaborator

@cacraigucar cacraigucar left a comment

Choose a reason for hiding this comment

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

I have reviewed up until the macmic section. I will continue my review from there next week.

Comment on lines +14 to +15
<scheme>diag_state_b4_phys_write</scheme> <!-- Diagnostics -->
<!-- energy and momentum fixer -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was check_tracers_init intentionally left out here? I see that it was moved to the "ac" section. Are the tracers the same value before and after coupling?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops, didn't mean to leave the check_tracers_init call out here. I can add it back in.

However, I also realized that this scheme seems more like a "timestep_init" routine, and so may not need to be in this file at all. Any thoughts @gold2718?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say 'probably' but I would need to look carefully to make sure that this routine only ever needs to be called at the beginning of a physics timestep. I think this may be part of a larger discussion we need to have about CAM physics. For historical purposes, we accumulate tracer (constituent) update instead of tendencies. Moving forwards, I see three options we (AMP) should explore:

  • Keep the current scheme (i.e., only track updated quantities)
  • Move to accumulating tendencies (for passing back to the dycore)
  • Have CAM accumulate the type of tracer feedback required by the dycore (e.g., updated quantity for FV, tendency for SE).

For now, remove it (i.e., think of it as becoming a timestep_init piece of some tracer scheme which is always present or something managed by CAM instead of the CCPP) and add it to the list of things we need to discuss.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like the check_tracers_init routine only computes the initial vertical integrals of the tracer quantities, and zeros out the associated boundary fluxes.  I could imagine this might require running the subroutine at the beginning of each physics group (e.g. physics_bc and physics_ac), as well as after any subroutine that modifies the vertical layering (e.g. pdel), but I don't see any other reason why it couldn't be run at the beginning of the model timestep, so I have removed it for now.

Also, I will create an issue for this repo to indicate that we need to discuss what quantity or quantities we will actually accumulate over the physics timestep, and thus pass on to the dycore.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is only one suite definition file which handles the timestep_init too. So I think it needs to remain and it will only have a timestep_init routine and no _run routine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

check_tracers_init is the "timestep_init" routine for the check_tracers_chng scheme, which is included in multiple locations in this file. So it won't be forgotten if removed, as it will be needed once we create the CCPP-ized version of check_tracers_chng.

suite_cam6.xml Outdated
Comment on lines 17 to 22
<scheme>calc_te_and_aam_budgets</scheme>
<scheme>check_energy_fix</scheme> <!--Only matters if dycore is FV (called "LR" in physics) or SE-->
<scheme>check_energy_cam_update_pre_chng</scheme> <!-- Placeholder for CAM ptend and diagnostic output calculations before "chng" call. -->
<scheme>check_energy_chng</scheme> <!-- Global integral checker required for certain diagnostic outputs -->
<scheme>check_energy_cam_update_post_chng</scheme> <!-- Placeholder for CAM ptend and diagnostic output calculations after "chng" call. -->
<scheme>calc_te_and_aam_budgets</scheme>
Copy link
Collaborator

Choose a reason for hiding this comment

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

In CAM6, calc_te_and_aam_budgets is being called with different inputs for the two calls. How will we handle this via the suite? There should probably be a comment added to each call with the different input indicated until this is addressed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am personally not sure what the best method is either for handling the different inputs, although it might be important to note that the actual calculations are done with the same inputs, it's just the diagnostic variable names in the history files that are different.

For now, though, I agree that including the outfield variable suffixes in the scheme comments would be beneficial, so I will go ahead and do so.

<scheme>convtran_cam_update</scheme> <!-- Placeholder for CAM ptend and diagnostic output calculations -->
<!-- **************************** -->
<scheme>check_energy_chng</scheme> <!-- Global integral checker required for certain diagnostic outputs -->
<scheme>check_tracers_chng</scheme> <!-- Global tracer mass checker that kills model if error is large enough. -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you actually update the tracers if they have not been initialized? (see my previous comment about possibly missing check_tracers_init)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Leaving out the check_tracers_init scheme was a mistake, although if we treat it as a "timestep_init" routine then its use may be implied, and thus wouldn't have to be included in this file anyways (I think?).

suite_cam6.xml Outdated
<scheme>set_dry_to_wet</scheme> <!-- Convert mixing ratios from dry air to air + water vapor -->
<scheme>grid_size</scheme> <!-- Calculate grid-cell size (not sure if this will be managed by registry/interface?) -->
<scheme>clubb_dtime_calc</scheme> <!-- Calculate CLUBB time-step as function of host model time-step -->
<scheme>exner_clubb</scheme> <!-- Calculate Exner function using CLUBB definition (should be manageable via the registry/meta file) -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need a virtual potential temperature calculation after the exner calculation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! I have added a new scheme to the suite labeled clubb_thermo_vars to represent that calculation (along with others I missed, like the calculation of the liquid potential temperature).

suite_cam6.xml Outdated
<scheme>clubb_dtime_calc</scheme> <!-- Calculate CLUBB time-step as function of host model time-step -->
<scheme>exner_clubb</scheme> <!-- Calculate Exner function using CLUBB definition (should be manageable via the registry/meta file) -->
<scheme>tropopause_findChemTrop</scheme> <!-- Calculate tropopause height -->
<scheme>clubb_vertical_grid_inv</scheme> <!-- Change order of vertical levels (should be manageable by registry/meta file) -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Before the vertical swapping, there are some difference calculations. Do we need them?

There are also some other quantities calculated (see the calculation for rho_ds_zt for example)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like those differences are related to the CLUBB vertical grid, so I went ahead and incorporated them in with the grid re-ordering to create a new, single clubb_vertical_grid_create_inv scheme.

I also added a new clubb_thermo_vars_grid scheme to incoporate all of the additional thermodynamic variable calculations that are done after the creation of the CLUBB vertical grids.

suite_cam6.xml Outdated
<scheme>exner_clubb</scheme> <!-- Calculate Exner function using CLUBB definition (should be manageable via the registry/meta file) -->
<scheme>tropopause_findChemTrop</scheme> <!-- Calculate tropopause height -->
<scheme>clubb_vertical_grid_inv</scheme> <!-- Change order of vertical levels (should be manageable by registry/meta file) -->
<scheme>clubb_calc_ustar</scheme> <!-- Only matters for single-column mode -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

The routine read_parameters_api is missing here. Should it be?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't sure about this one, as the call looks like it is just setting default values for these parameters, and so I could imagine that is manageable by the registry and a "timestep_init" routine. However, if not then I am happy to add it to this suite file.

suite_cam6.xml Outdated
Comment on lines 61 to 62
<scheme>setup_grid_heights_api</scheme> <!-- Re-calculate CLUBB grid heights -->
<scheme>setup_parameters_api</scheme> <!-- Re-calculate grid-dependent CLUBB parameters -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

A general question is whether we should make a wrapper around the CLUBB library routines. I don't think we want to go into the CLUBB library itself and make modifications, but rather have one or more interfaces to the CLUBB routines we call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This might be worth discussing, as I could imagine both pros and cons. On the one hand I can see the argument that the suite definition file should include all the separate CAM schemes (without additional interface layers), as that way a user (either a scientist or engineer) can quickly see exactly what CAM is doing, and in what order.

On the other hand, the CLUBB interface is quite tricky because several sections of calculations depend on namelist variables (like do_rainturb), and so if we included every one of those options as "execute" statements it could possibly become difficult to read the suite definition file. I guess we should wait and see if @gold2718 has any strong opinions on the matter.

suite_cam6.xml Outdated
<scheme>clubb_calc_ustar</scheme> <!-- Only matters for single-column mode -->
<scheme>setup_grid_heights_api</scheme> <!-- Re-calculate CLUBB grid heights -->
<scheme>setup_parameters_api</scheme> <!-- Re-calculate grid-dependent CLUBB parameters -->
<scheme>zt2zm_convert</scheme> <!-- Transition variables on thermodynamic levels to momentum levels (manageable by registry/meta file?) -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this what had the comment "Surface fluxes provided by host model"? If not, do we need to add this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Originally yes, but given that the scheme name is vague I decided to just add a new scheme labeled clubb_calc_input_vars representing the surface fluxe calculations and all other possible input variable calculations that need to be done that are not already accounted for in other schemes.

suite_cam6.xml Outdated
<subcycle loop="clubb_time_substep"> <!-- CLUBB sub-step loop (number of iterations should be calculated in the "clubb_dtime_calc" scheme), CAM6 var = 'clubb_nadv' -->
<scheme>stats_begin_timestep_api</scheme> <!-- Only matters if "l_stats" is True -->
<scheme>advance_clubb_core_api</scheme> <!-- Actual CLUBB parameterization scheme -->
<scheme>update_xp2_mc_api</scheme> <!-- Only matters if "do_rainturb" is True -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

After this call, the original code has "update turbulent moments". Should this be added? If so, it too is only if do_rainturb is True

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed the name of the scheme to update_xp2_mc_intr to indicate that the "scheme" is itself just a wrapper around the CLUBB API routine. I figure that was an ideal solution that allows us to not modify CLUBB itself, but also doesn't require us to create an entirely new scheme for a relatively simple calculation that is only done in one location in the entire model.

suite_cam6.xml Outdated
<scheme>stats_begin_timestep_api</scheme> <!-- Only matters if "l_stats" is True -->
<scheme>advance_clubb_core_api</scheme> <!-- Actual CLUBB parameterization scheme -->
<scheme>update_xp2_mc_api</scheme> <!-- Only matters if "do_rainturb" is True -->
<scheme>calculate_thlp2_rad_api</scheme> <!-- Only matters if "do_cldcool" is True -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

A couple of additional values are calculated after the call. They could be factored into the CCPP wrapper routine if we don't call CLUBB directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, hence I provided the same solution as above (slightly re-name the scheme to indicate it is a wrapper around the actual CLUBB code that includes these additional calculations).

suite_cam6.xml Outdated
<scheme>stats_end_timestep_clubb</scheme> <!-- Only matters if "l_stats" is True -->
</subcycle>
<scheme>zm2zt_convert</scheme> <!-- Transition variables on momentum levels to thermodynamic levels (manageable by registry/meta file?) -->
<scheme>clubb_var_vert_rev</scheme> <!-- Change order of vertical levels for CLUBB output variables back to host model ordering (should be manageable via the registry/meta file) -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a couple of calculated variables in with the vertical swapping (mean_rt and pdfp_rtp2)
Also "compute static energy using CLUBB's variables"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will continue my review from this point next week

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a new clubb_calc_output_vars scheme which represents the calculation of these additional variables.

suite_cam6.xml Outdated
<scheme>conv_cond_detrain_calc</scheme> <!-- Calculate the host model cloud condensate and energy tendencies produced by convective detrainment -->
<scheme>conv_cond_detrain_cam_update</scheme> <!-- Placeholder for CAM ptend and diagnostic output calculations -->
<scheme>set_wet_to_dry</scheme> <!-- Convert mixing ratios from air + water vapor to dry air -->
<scheme>clubb_diag_output</scheme> <!-- Calculate numerous different CLUBB diagnostic variables, and output them to history file -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to worry about scaling tendencies for substeps (physics_ptend_scale)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My understanding is that the subcycle methodology would manage that for us (but @gold2718 can correct me if I am wrong).

suite_cam6.xml Outdated
<scheme>clubb_diag_output</scheme> <!-- Calculate numerous different CLUBB diagnostic variables, and output them to history file -->
<!-- **************************** -->
<scheme>check_energy_chng</scheme> <!-- Global integral checker required for certain diagnostic outputs -->
<subcol gen="subcol_generator_name" avg="subcol_averager_name">
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 a question for Steve - Should this be more generic (and not "subcol")? Could this logic be expanded to handle different types of "substitutions"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Before attempting a generalization, a set of scenarios is helpful. Can you think of any use case which would fit into this type of configuration?

suite_cam6.xml Outdated
<process_split>
<!-- MG aerosol microphysics -->
<!-- **************************** -->
<scheme>microp_aero_run</scheme> <!-- Some, but not all, tendencices from here are added to the state before microp. This is done via pbuf.-->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you not break this into the separate calls? (hetfrz_classnuc_cam_save_cbaero, and nucleate_ice_cam_calc to name a couple). An alternate question, is why break CLUBB into so many pieces? A more general question for all of these sections, is what is the independent "swappable" granularity for each parameterization? There will be "prep and finalize" code which will be dependent for CAM, but I'm wondering if we are getting the correct granularity with each section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The short answer is because I considered mircop_aero_run to be a parameterizaton, as opposed to an interface to a parameterization (whereas clubb_tend_cam is clearly an interface to the CLUBB paramterization).

However, I do agree that these sort of decisions are fairly subjective, and so I am happy to discuss where the granularity "cut-off" should be for any particular physics parameterization/interface.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that it needs to be broken up as it makes it really hard to switch other parts of the code (e.g, some desired uses of CARMA) to match. I understand that this is tricky right now because, for instance, what should you do with all the rad_cnst_xxx stuff? We could postpone this for now and add a comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I went ahead and split the microp_aero_run scheme into its corresponding sub-sections and subroutines. I imagined one could combine all of the rad_cnst_xxx calls into a single aero_get_num_mmr scheme, so I've gone ahead and done so, at least for now.

suite_cam6.xml Outdated
<scheme>micro_mg_get_cols3_0</scheme> <!-- Determine atmospheric columns used by the MG2 microphysics schemes -->
<scheme>calc_incloud_LWP</scheme> <!-- Calculate in-cloud Liquid Water Path (LWP) before MG2 is run -->
<scheme>micro_calc_tropopause</scheme> <!-- Calculate the cold-point tropopause for use by the microphysics -->
<scheme>micro_mg_tend2_0</scheme> <!-- Actual MG2 cloud micro-physics parameterization scheme -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assume that this will be what is stored in the MG repo. That means that running MG2 will be a call to micro_mg_tend3_0 (there is no 2 version any longer)

Will continue review from here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching this! It should be fixed now.

Copy link
Contributor

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Getting better. I think there is a typo in a comment.

suite_cam6.xml Outdated
<scheme>modal_aero_calcsize_diag</scheme> <!-- Only if climatological aerosols are used instead of prognostic aerosols -->
<!-- Modal Aerosol water uptake --> <!-- Only if climatological aerosols are used instead of prognostic aerosols -->
<!-- **************************** -->
<scheme>tropopause_find</scheme> <!-- CalManaged by host model?culate tropopause height -->
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment seems to have gotten mangled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for finding this typo!  The comment has now been fixed.

suite_cam6.xml Outdated
<process_split>
<!-- MG aerosol microphysics -->
<!-- **************************** -->
<scheme>microp_aero_run</scheme> <!-- Some, but not all, tendencices from here are added to the state before microp. This is done via pbuf.-->
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that it needs to be broken up as it makes it really hard to switch other parts of the code (e.g, some desired uses of CARMA) to match. I understand that this is tricky right now because, for instance, what should you do with all the rad_cnst_xxx stuff? We could postpone this for now and add a comment.

Copy link
Contributor

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Looking better, forgot to ask about ptend removal last time.

suite_cam6.xml Outdated
<scheme>momentum_flux_calc</scheme> <!-- Calculate momentum fluxes/wind tendencies -->
<scheme>energy_change</scheme> <!-- Calculate atmospheric energy change -->
<scheme>energy_fixer</scheme> <!-- Ensure energy conservation -->
<scheme>gw_cam_update</scheme> <!-- Placeholder for CAM gravity wave ptend and diagnostic output calculations -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the ptend part of these comments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've now removed any reference to "ptend" from the file, with the one exception being the co2_cycle_set_ptend scheme name, given that I wanted to keep the name of the scheme the same as the actual subroutine in CAM. However, I imagine this can be changed in the future.

suite_cam6.xml Outdated
<scheme>micro_calc_tropopause</scheme> <!-- Calculate the cold-point tropopause for use by the microphysics -->
<scheme>micro_mg_tend3_0</scheme> <!-- Actual MG2 cloud micro-physics parameterization scheme -->
<scheme>mg_calc_outputs</scheme> <!-- Calculate additional MG2 output variables and diagnostics -->
<scheme>calc_atm_density</scheme> <!-- Calculate atmospheric density -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if this is including the calculations for the in-cloud mixing ratios or not (icimrst is an example)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This calc_atm_density scheme represents the calculation of rho (which is air density) on line 2981 in micro_mg_cam.F90, which appears to be entirely grid-column (i.e. state) inputs. Given that rho is calculated in multiple different locations, I figured this scheme could be included in the CCPP “state converters” utility, at least for dry air.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, I was assuming that the in-cloud diagnostics would be included in the mg_calc_outputs scheme, in case you were curious.

suite_cam6.xml Outdated
<scheme>calc_atm_density</scheme> <!-- Calculate atmospheric density -->
<!-- This section of code could likely be grouped together as one scheme (at least partially)-->
<!-- +++++++++++++++++++++++++ -->
<scheme>size_dist_param_liq</scheme> <!-- Calculate size distribution of cloud droplets -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does rho need to be calculated prior to this call?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The two size_dist_param_liq and subsequent micro_eff_radius_liq calls have ncic_grid set to 1 and nc_grid/liqclf_grid respectively (the comments should probably reflect these input changes)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As mentioned above, “rho” is actually density, and so is managed by the calc_atm_density scheme.

Also, I have added a new size_dist_param_liq_const scheme to represent the call to the size_dist_param_liq subroutine when ncic is a global constant (although I am not completely sure if this is the best method to handle this particular issue, it should hopefully work for now).

Comment on lines +14 to +15
<scheme>diag_state_b4_phys_write</scheme> <!-- Diagnostics -->
<!-- energy and momentum fixer -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is only one suite definition file which handles the timestep_init too. So I think it needs to remain and it will only have a timestep_init routine and no _run routine.

suite_cam6.xml Outdated
<scheme>micro_eff_radius_liq</scheme> <!-- Calculate the effective radius of a given cloud droplet distribution -->
<scheme>micro_eff_radius_rain</scheme> <!-- Calculate the effective radius of rain (assumed size distribution?) -->
<scheme>micro_eff_radius_snow</scheme> <!-- Calculate the effective radius of snow (assumed size distribution?) -->
<scheme>calc_niic_grid</scheme> <!-- Calculate niic (number of cloud ice crystals?) in grid cell -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does graupel need to be included as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not completely sure, as it is technically only exists in MG version 3 (or greater). However, given that the code is already using the MG3 interface, I agree that it makes sense to include graupel, so I have gone-ahead and added a micro_eff_radius_graupel scheme.

suite_cam6.xml Outdated
Comment on lines 127 to 147
<execute test="use_SILHS" default=".false." >
<!-- **************************** -->
<!-- SILHS subcolumn covariance -->
<!-- **************************** -->
<scheme>calc_total_moisture</scheme> <!-- Combine vapor and condensate (manageable by registry/framework)? -->
<scheme>calc_dry_static_density</scheme> <!-- Calculate the dry static density -->
<scheme>convert_omega_to_w</scheme> <!-- Convert pressure velocity to vertical velocity, manageable by registry/framework? -->
<scheme>convert_dse_to_temp</scheme> <!-- Convert dry static energy (DSE) to air temperature, manageable by registry/framework? -->
<scheme>exner_silhs</scheme> <!-- Calculate the Exner function according to SILHS (should be the same as CLUBB, but possible issues) -->
<scheme>thl_calc</scheme> <!-- Calculate liquid water potential temperature (theta-l) -->
<scheme>silhs_var_vert_inv</scheme> <!-- Change order of vertical levels for SILHS input variables, and add ghost point (should be manageable via the registry/meta file) -->
<scheme>silhs_get_subcol_wgts</scheme> <!-- Collect subcolumn weights for SILHS -->
<scheme>lh_microphys_var_covar_driver_api</scheme> <!-- Actual SILHS sub-column parameterization -->
<scheme>zero_upper_level</scheme> <!-- Set covariances above SILHS max altitude to zero -->
<!-- **************************** -->
<scheme>subcol_SILHS_fill_holes_conserv_calc</scheme> <!-- Prevent holes (values less than qmin) using mass-conserving adjustments -->
</execute>
<scheme>massless_droplet_destroyer</scheme> <!-- Remove massless droplets, doesn't influences only grid-scale tendencies -->
<execute test="use_SILHS" default=".false." >
<scheme>subcol_SILHS_hydromet_conc_tend_lim</scheme> <!-- Place limits on hydrometeor drop-size -->
</execute>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tend to think that SILHS should be a single scheme. It can not be run without CLUBB (at least I don't think so), so it's likelihood of being shared is low. I could see us swapping different versions of SILHS, but not individual pieces within it.

I did not verify this section, but if it is decided that SILHS does need to be put into individual schemes, I will review the details of this section.

suite_cam6.xml Outdated
Comment on lines 45 to 77
<subcycle loop="cloud_macmic_num_steps"> <!-- Cloud macrophysics-microphysics loop -->
<!-- CLUBB turbulence, convection, and macrophysics -->
<!-- **************************** -->
<!--This section will likely require additional clean-up,
as many of these routines are simply unit or vertical
grid conversions, or the re-naming and re-setting
of host model inputs. Thus they can likely be
managed by the framework itself, and won't need
their own scheme calls.-->
<scheme>set_dry_to_wet</scheme> <!-- Convert mixing ratios from dry air to air + water vapor -->
<scheme>grid_size</scheme> <!-- Calculate grid-cell size (not sure if this will be managed by registry/interface?) -->
<scheme>clubb_dtime_calc</scheme> <!-- Calculate CLUBB time-step as function of host model time-step -->
<scheme>exner_clubb</scheme> <!-- Calculate Exner function using CLUBB definition (should be manageable via the registry/meta file) -->
<scheme>clubb_thermo_vars</scheme> <!-- Calculate various thermodynamic variables needed for CLUBB (Theta-V, Theta-L, etc.) -->
<scheme>tropopause_findChemTrop</scheme> <!-- Calculate tropopause height -->
<scheme>clubb_vertical_grid_create_inv</scheme> <!-- Create CLUBB vertical grids, which is in the opposite order of CAM -->
<scheme>clubb_thermo_vars_grid</scheme> <!-- Calculate additional thermodynamic variables that are on the CLUBB vertical grid -->
<scheme>clubb_calc_ustar</scheme> <!-- Only matters for single-column mode -->
<scheme>setup_grid_heights_api</scheme> <!-- Re-calculate CLUBB grid heights -->
<scheme>setup_parameters_api</scheme> <!-- Re-calculate grid-dependent CLUBB parameters -->
<scheme>zt2zm_convert</scheme> <!-- Transition variables on thermodynamic levels to momentum levels (manageable by registry/meta file?) -->
<scheme>clubb_calc_input_vars</scheme> <!-- Calculate additional CLUBB input variables -->
<scheme>clubb_var_vert_inv</scheme> <!-- Change order of vertical levels for CLUBB input variables -->
<subcycle loop="clubb_time_substep"> <!-- CLUBB sub-step loop (number of iterations should be calculated in the "clubb_dtime_calc" scheme), CAM6 var = 'clubb_nadv' -->
<scheme>stats_begin_timestep_api</scheme> <!-- Only matters if "l_stats" is True -->
<scheme>advance_clubb_core_api</scheme> <!-- Actual CLUBB parameterization scheme -->
<scheme>update_xp2_mc_intr</scheme> <!-- Only matters if "do_rainturb" is True -->
<scheme>calculate_thlp2_rad_intr</scheme> <!-- Only matters if "do_cldcool" is True -->
<scheme>stats_end_timestep_clubb</scheme> <!-- Only matters if "l_stats" is True -->
</subcycle>
<scheme>zm2zt_convert</scheme> <!-- Transition variables on momentum levels to thermodynamic levels (manageable by registry/meta file?) -->
<scheme>clubb_calc_output_vars</scheme> <!-- Calculate additional CLUBB output variables -->
<scheme>clubb_var_vert_rev</scheme> <!-- Change order of vertical levels for CLUBB output variables back to host model ordering -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the CLUBB library is an "external" to us, I think it should probably be a single scheme. If in the future, UWM separates out various process, then I could see creating separate schemes with the individual functionality.

Comment on lines +168 to +172
<!-- **************************** -->
<!-- Aerosol wet deposition -->
<!-- **************************** -->
<scheme>modal_aero_calcsize_sub</scheme> <!-- Calculate aerosol size distribution parameters -->
<scheme>modal_aero_wateruptake_dr</scheme> <!-- Repeat "Modal Aerosol water uptake" schemes shown above -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

This appears to be a duplication of the above lines from 156-167. I like the collapsing that you did here (and was going to comment on the fine granularity). I suggest that lines 156-167 be deleted.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have commented out the schemes and added a new modal_aero_wateruptake_dr scheme call, similar to what was done to clean-up RRTMG and CLUBB.

@nusbaume
Copy link
Collaborator Author

I've implemented the changes that were discussed during the hangout this morning, and have added a new suite_cam6_silhs.xml file which contains the SILHS schemes. However, unless you are examining the SILHS implementation itself, feel free to only review the original suite_cam6.xml file, as I will attempt to keep the two files exactly the same outside of SILHS. Thanks!

Copy link
Contributor

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

I think this is as good as we can do without scientist review.

Copy link
Collaborator

@cacraigucar cacraigucar left a comment

Choose a reason for hiding this comment

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

Finished reviewng tphysbc, so sending this along

Comment on lines +163 to +169
<subcycle loop="aerosol_dist_modes"> <!-- Loop over aerosol size distribution modes, CAM6 var = 'ntot_amode' -->
<subcycle loop="aerosol_phase"> <!-- Loop over aerosol phase types (interstitial vs cloud-borne), CAM6 var = 'lphase' -->
<scheme>aero_fact_calc</scheme> <!-- Calculate aerosol activation fraction (f_act_conv) depending on aerosol type -->
<scheme>wetdepa_v2</scheme> <!-- Actual aerosol wet deposition parameterization -->
<scheme>wetdep_diag_output</scheme> <!-- Calculate numerous different CLUBB diagnostic variables, and output them to history file -->
</subcycle>
</subcycle>
Copy link
Collaborator

Choose a reason for hiding this comment

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

If wetdep becomes an attribute of a chemistry user defined type, would the subcycles would go away?

Comment on lines +161 to +162
<scheme>wetdep_prec_calc</scheme> <!-- Calculate precipitation amount that impacts wet deposition -->
<scheme>coarse_fact_calc</scheme> <!-- Calculate "f_act_conv" for coarse mode aerosols (note here the comments don't match the variables) -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these be combined into a "wetdep_pre" routine? Does it make sense to do so?

Comment on lines +170 to +171
<scheme>ma_convproc_intr</scheme> <!-- Convective in-cloud aerosol removal parameterization, only used if convproc_do_aer is True -->
<scheme>set_srf_wetdep</scheme> <!-- Add aerosol deposition rates to host model surface fluxes -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these be combined as wetdep_post?

Comment on lines +200 to +205
<subcycle loop="rad_active_cnst"> <!-- Loop over radiatively active constituents, CAM6 var = 'n_diag' -->
<scheme>rrtmg_state_update</scheme> <!-- Add shortwave constiuent concentrations to RRTMG state structure -->
<scheme>aer_rad_props_sw</scheme> <!-- Gather/calculate aerosol shortwave optical properties (potentially provided by registry/framework?) -->
<scheme>rad_rrtmg_sw</scheme> <!-- Actual RRTMG shortwave radiation parameterization -->
<scheme>rad_sw_diag_output</scheme> <!-- Calculate shortwave diagnostic variables, and output them to history file -->
</subcycle>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this subcycle (and the subcycle following for longwave) go away with an attribute saying that the variable is shortwave radiatively active?

Comment on lines +178 to +179
<!-- RRTMG radiation -->
<!-- **************************** -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

For RRTMG - it might be useful to see the one or two RRTMG routines which have already been ccpp'ized and see how our implementation might fit around them.

Comment on lines +215 to +223
<scheme>rad_qdp_q_calc</scheme> <!-- Convert Q*dp to Q (potentially managed by registry/framework?) -->
<scheme>rad_data_write</scheme> <!-- Calculate additional radiation diagnostics, and output them to history file -->
<scheme>radheat_tend</scheme> <!-- Calculate net radiative heating tendencies -->
<scheme>radheat_diag_output</scheme> <!-- Calculate radiative heating diagnostic variables, and output them to history file -->
<scheme>rad_q_qdp_calc</scheme> <!-- Convert Q to Q*dp (potentially managed by registry/framework?) -->
<scheme>set_srf_net_sw</scheme> <!-- Add net shortwave surface flux to host model surface fluxes -->
<!-- **************************** -->
<scheme>check_energy_chng</scheme> <!-- Global integral checker required for certain diagnostic outputs -->
<scheme>tropopause_output</scheme> <!-- Calculate tropopause diagnostic variables, and output them to history file -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this all be combined in radiation_post?

@gold2718
Copy link
Contributor

@cacraigucar, these are all great questions but I think they are also questions for the scientists. For instance, combining routines (e.g., wetdep_prec_calc and scheme>coarse_fact_calc) only makes sense if it never makes sense to run one without the other.
Also, I do not see wetdep becoming part of chemistry. Chemistry will generate the aerosols, wet physics will scavenge them. All the endless CCPP Framework discussions are about how to do that handshake.

Also, you requested changes but it is not clear to me what those are (as opposed to questions).

@cacraigucar
Copy link
Collaborator

This time around I hit comment. The "requested changes" was from a previous review (and Jesse may have addressed it/them).

@gold2718
Copy link
Contributor

If your concerns have been addressed, you need to release the change request.

Copy link
Collaborator

@cacraigucar cacraigucar left a comment

Choose a reason for hiding this comment

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

I have completed my review of the CAM6 suite (I do not plan to review the SILHS suite at this time).

After my last few comments are addressed, I should be able to approve this PR.

Comment on lines +249 to +250
<!-- Atmospheric Chemistry (MOZART), only matters if chem_is_active is True -->
<!-- **************************** -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume some/all? chemistry will eventually be replaced by MICM?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but until they present us with something, we cannot make any decisions on how to use it or what needs replacing or rearranging. For now, we need to leave this here even though we will probably not end up porting many of these routines.

Comment on lines +294 to +295
<!-- Aerosol dry deposition -->
<!-- **************************** -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the ACOM group is doing a whole aerosol project that they are targeting to run using CCPP (part of Musica). Would it replace this whole section?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think these are different functions. Musica would create the aerosols but it is other physics processes that remove them from the atmosphere. That is what is going on here.

Comment on lines +332 to +333
<!-- Generic gravity wave scheme order -->
<!-- +++++++++++++++++++++++++ -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't Julio working on a CCPP version of gravity wave? If so, can its suite be put here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is way too soon for that. When he is ready, his version could replace the current GW code in our suite.

Comment on lines +343 to +344
<!-- +++++++++++++++++++++++++ -->
<!-- **************************** -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is qbo_relax missing here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The qbo_relax subroutine only does something if WACCM is enabled (which will be a separate SDF). Otherwise it just outputs tendencies of zero. Given this, I decided not to include it in the CAM6 files.

@nusbaume nusbaume merged commit 800709f into ESCOMP:master Mar 27, 2020
@nusbaume nusbaume deleted the cam6_suite_file branch July 26, 2021 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants