Skip to content

Add the subroutines of GF cps for CCPP-FV3#139

Merged
climbfuji merged 4 commits into
NCAR:masterfrom
haiqinli:gfcps
Oct 10, 2018
Merged

Add the subroutines of GF cps for CCPP-FV3#139
climbfuji merged 4 commits into
NCAR:masterfrom
haiqinli:gfcps

Conversation

@haiqinli
Copy link
Copy Markdown
Collaborator

@haiqinli haiqinli commented Aug 8, 2018

Create the new branch of gfcps to include the subroutines of Grell-Freitas cps for CCPP-FV3.

Comment thread input.nml
random_clds = .true.
trans_trac = .true.
cnvcld = .true.
imfshalcnv = 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.

I am not sure if you are supposed to change the default model config. Maybe @grantfirl has a better idea on how to create a different input.nml that uses GF instead of SAS (and a corresponding suite definition file for both FV3 and SCM).

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.

Which brings up the question why a copy of input.nml resides in ccpp-physics - it shouldn't be there. @grantfirl is it actually used? Not by FV3 at least, but maybe by SCM?

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.

To follow up on this, @haiqinli, Dom has (rightly) moved the input.nml used in the SCM from the ccpp-physics repository to the gmtb-scm repository. It is now located in gmtb-scm/scm/etc/case_config in the master branch. Also as Dom suggested, for testing the GF scheme, it is best to create a copy of input.nml for your testing, perhaps input_GF.nml. The original represents the operational namelist values and should stay that way. Then, when you go to run the SCM, you'll also want to modify the case configuration namelists to point to your modified input_GF.nml. For example, if you run the TWP-ICE case, make a copy of gmtb-scm/scm/etc/case_config/twpice.nml, rename it twpice_GF.nml (or similar) and edit the physics_nml variable within to point to your new input_GF.nml file.

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.

As far as I can see, this has not been addressed yet.

Comment thread physics/cu_gf_deep.F90

!> \brief brief description of the subroutine
!!
!! \section arg_table_sasas_deep_init argument table
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.

This should be "arg_table_cu_gf_deep_init"

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.

Or, even better, since these routines are not CCPP entry points, no argument table is needed - delete lines 46-47

Comment thread physics/cu_gf_deep.F90

!> \brief brief description of the subroutine
!!
!! \section arg_table_sasas_deep_finalize argument table
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.

This should be "arg_table_cu_gf_deep_finalize"

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.

Or, even better, since these routines are not CCPP entry points, no argument table is needed - delete lines 54-55

Comment thread physics/cu_gf_driver.F90
use machine , only: kind_phys
use cu_gf_deep, only: cu_gf_deep_run,neg_check,autoconv,aeroevap
use cu_gf_sh , only: cu_gf_sh_run

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.

missing:

private

public :: cu_gf_driver_init, cu_gf_driver_run, cu_gf_driver_finalize

Comment thread physics/cu_gf_driver.F90
@@ -0,0 +1,786 @@
!
module cu_gf_driver
use physcons , g => con_g, cp => con_cp, xlv => con_hvap, r_v => con_rv
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.

These physical constants should come in via the subroutine's argument lists.

Comment thread physics/cu_gf_driver.F90
!! | hfx2 | kinematic_surface_upward_sensible_heat_flux | kinematic surface upward sensible heat flux | K m s-1 | 1 | real | kind_phys | out | F |
!! | qfx2 | kinematic_surface_upward_latent_heat_flux | kinematic surface upward latent heat flux | kg kg-1 m s-1 | 1 | real | kind_phys | out | F |
!! | clw | convective_transportable_tracers | array to contain cloud water and other convective trans. tracers | kg kg-1 | 3 | real | kind_phys | none | F |
!! | pbl | atmosphere_boundary_layer_thickness | PBL thickness | m | 1 | real | kind_phys | out | F |
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.

adjust intents

Comment thread physics/cu_gf_driver.F90
pbl,ud_mf,dd_mf,dt_mf,cnvw,cnvc,errmsg,errflg)
! pbl,ud_mf,dd_mf,dt_mf,gdc,gdc2,cnvw,cnvc,ishal_cnv)
!-------------------------------------------------------------
implicit none
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.

preferably all interface variables (i.e. those coming in via the argument list) are listed first, followed by local parameters and variables

subroutine cu_gf_driver_post_finalize()
end subroutine cu_gf_driver_post_finalize

!> \section arg_table_cu_gf_driver_post_run Argument Table
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.

instead of having Model, Stateout, Grid, Tbd in the argument list (of which only Tbd and Stateout are used anyway), it would be better to pass in the components phy_f3d(:,:,5), phy_f3d(:,:,6), gt0(:,:) and gq0(:,:,1) using their respective standard names (see GFS_typedefs.F90) and giving them meaningful local names.

!> \section arg_table_cu_gf_driver_pre_run Argument Table
!! | local_name | standard_name | long_name | units | rank | type | kind | intent | optional |
!! |----------------|--------------------------------------------------------|--------------------------------------------------------------------------|---------------|------|-------------------|-----------|--------|----------|
!! | Model | FV3-GFS_Control_type | Fortran DDT containing FV3-GFS model control parameters | DDT | 0 | GFS_control_type | | in | F |
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.

see above comment for cu_gf_driver_post_run (replace GFS derived data types with individual components)

Comment thread physics/cu_gf_deep.F90
@@ -0,0 +1,4754 @@
module cu_gf_deep
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.

this is a pretty minor request, but it would be great to declare all variables/subroutines as private and only those variables and subroutines used as public. For instance, adding

private

public :: LIST OF VARIABLES AND SUBROUTINES THAT GET USED BY THE CU_GF_DRIVER MODULE

@haiqinli
Copy link
Copy Markdown
Collaborator Author

haiqinli commented Aug 9, 2018 via email

@climbfuji
Copy link
Copy Markdown
Collaborator

Sure, no problems. I am still working my way through, but all in all your work looks great.

Comment thread physics/cu_gf_sh.F90

!> \brief brief description of the subroutine
!!
!! \section arg_table_sasas_deep_init argument table
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.

same as for cu_gf_deep, no variable table needed here

Comment thread physics/cu_gf_sh.F90

!> \brief brief description of the subroutine
!!
!! \section arg_table_sasas_deep_finalize argument table
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.

no variable table needed here

Comment thread physics/cu_gf_sh.F90
@@ -0,0 +1,866 @@
! module cup_gf_sh will call shallow convection as described in grell and
! freitas (2016). input variables are:
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.

As for cu_gf_deep: this is a pretty minor request, but it would be great to declare all variables/subroutines as private and only those variables and subroutines used as public. For instance, adding

private

public :: LIST OF VARIABLES AND SUBROUTINES THAT GET USED BY THE CU_GF_DRIVER MODULE

Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Great job overall, I noted a few issues and a few nice to haves.

@climbfuji
Copy link
Copy Markdown
Collaborator

@climbfuji
Copy link
Copy Markdown
Collaborator

Superseded by #166.

@climbfuji climbfuji merged commit f1fa403 into NCAR:master Oct 10, 2018
SamuelTrahanNOAA added a commit to SamuelTrahanNOAA/ccpp-physics that referenced this pull request May 17, 2022
The change gets around snow initialization with the use of RUC LSM
scrasmussen pushed a commit to scrasmussen/ccpp-physics that referenced this pull request Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants