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

Constituents #206

Merged
merged 34 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2d48f66
Add call to cam_final in ModelFinalize
Oct 21, 2022
b2ee696
Uncomment CCPP version of constituent information gathering code and …
Jan 2, 2023
147169b
Update CCPP Framework external
Jan 2, 2023
c023d6f
move composition-dependent variables to registry
Jan 6, 2023
fbc54f6
syntax fix; couple constituent fixes
Jan 6, 2023
502a7e5
Merge pull request #1 from peverwhee/constituents_court
gold2718 Jan 9, 2023
f703d54
Remove bad import, initialize physconst namelist variables
Jan 9, 2023
582589c
fix stdnames, move air_composition_init, add nl xml files for null dy…
Jan 12, 2023
076bc79
fix stdnames, remove allocates from air_composition_init
Jan 14, 2023
a8a5394
add script to convert inputnames to stdnames in netcdf file; fixes an…
Feb 3, 2023
cdccfef
fix non-declared local variable
Feb 4, 2023
aa68644
update externals; initialize constituent_index
Feb 7, 2023
bf9b08e
use full constituent array rather than advected array
Feb 7, 2023
0126363
code clean-up
Feb 7, 2023
1d11cf8
fix stdnames converter
Feb 7, 2023
0d519b8
address reviewer comments; remove indices from cam_constituents; don'…
Feb 21, 2023
4a21611
updates for se dycore
Mar 1, 2023
48d8e35
updates to ic tests
Mar 1, 2023
cae253f
merge to head of development
Mar 2, 2023
818617b
fix unit tests and amend python script
Mar 2, 2023
dc004f2
update framework external
Mar 2, 2023
7b9fb3c
add python 3.11 unit tests
Mar 2, 2023
cf3e747
address reviewer comments
Mar 2, 2023
0571130
address reviewer comments
Mar 7, 2023
3106e8b
address some comments; remove composition_dependent_init; use stdname…
Mar 14, 2023
6862f3b
address further review comments
Mar 22, 2023
9433fcc
more changes
Mar 22, 2023
92639bf
address comments; fix build error
Mar 23, 2023
00f4423
fix indexing error
Mar 30, 2023
98d917f
do not read from file when initialized in init phase
Apr 11, 2023
4e5e1f0
address reviewer comments
Apr 13, 2023
ef5223d
update externals
May 11, 2023
4f19dc6
remove hard-coded three constituents from se dp_coupling
May 11, 2023
af08f73
remove declaration for unused variable
May 15, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/python_unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
strategy:
matrix:
#All of these python versions will be used to run tests:
python-version: ["3.7", "3.8", "3.9", "3.10"]
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
fail-fast: false
steps:
# Acquire github action routines:
Expand Down
4 changes: 2 additions & 2 deletions Externals_CAM.cfg
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[ccpp-framework]
local_path = ccpp_framework
protocol = git
repo_url = https://github.com/gold2718/ccpp-framework
tag = CPF_0.2.032
repo_url = https://github.com/peverwhee/ccpp-framework
tag = CPF_0.2.039
required = True

[cosp2]
Expand Down
58 changes: 50 additions & 8 deletions cime_config/cam_build_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,34 @@ def new_entry_from_xml(item):
# end if
return new_entry

###############################################################################
def clean_xml_text(item):
###############################################################################
"""Return a 'clean' (stripped) version of <item>.text or an empty
string if <item>.text is None or not a string-type variable

doctests

1. Test that the function works as expected when passed a string.
>>> test_xml = ET.Element("text")
>>> test_xml.text = " THIS IS A test "
>>> clean_xml_text(test_xml)
'THIS IS A test'

2. Verify that the function returns an empty string when not passed a string.
>>> test_xml = ET.Element("text")
>>> test_xml.text = 2
>>> clean_xml_text(test_xml)
''

"""
itext = item.text
iret = ""
if isinstance(itext, str):
iret = itext.strip()
# end if
return iret

class FileStatus:
"""Class to hold full path and SHA hash of a file"""

Expand Down Expand Up @@ -219,13 +247,14 @@ def __init__(self, build_cache):
elif item.tag == 'config':
self.__config = item.text
elif item.tag == 'reg_gen_file':
self.__reg_gen_files.append(item.text.strip())
self.__reg_gen_files.append(clean_xml_text(item))
elif item.tag == 'ic_name_entry':
stdname = item.get('standard_name')
if stdname not in self.__ic_names:
self.__ic_names[stdname] = []
# end if
self.__ic_names[stdname].append(item.text.strip())
itext = clean_xml_text(item)
self.__ic_names[stdname].append(itext)
else:
emsg = "ERROR: Unknown registry tag, '{}'"
raise ValueError(emsg.format(item.tag))
Expand All @@ -248,16 +277,29 @@ def __init__(self, build_cache):
new_entry = new_entry_from_xml(item)
self.__xml_files[new_entry.key] = new_entry
elif item.tag == 'scheme_namelist_meta_file':
self.__scheme_nl_metadata.append(item.text.strip())
if isinstance(item.text, str):
if item.text:
self.__scheme_nl_metadata.append(item.text.strip())
# end if
# end if
elif item.tag == 'scheme_namelist_groups':
group_list = [x for x in
item.text.strip().split(' ') if x]
group_list = []
if isinstance(item.text, str):
if item.text:
group_list = [x for x in
item.text.strip().split(' ') if x]
# end if
# end if
self.__scheme_nl_groups = group_list
elif item.tag == 'preproc_defs':
self.__preproc_defs = item.text.strip()
self.__preproc_defs = clean_xml_text(item)
elif item.tag == 'kind_type':
kname, ktype = item.text.strip().split('=')
self.__kind_types[kname.strip()] = ktype.strip()
if isinstance(item.text, str):
if item.text:
kname, ktype = item.text.strip().split('=')
self.__kind_types[kname.strip()] = ktype.strip()
# end if
# end if
else:
emsg = "ERROR: Unknown CCPP tag, '{}'"
raise ValueError(emsg.format(item.tag))
Expand Down
1 change: 0 additions & 1 deletion cime_config/cam_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ def __init__(self, case, case_log):
#Add the default host model namelists:
self._add_xml_nml_file(cime_conf_path, "namelist_definition_cam.xml")
self._add_xml_nml_file(data_nml_path, "namelist_definition_physconst.xml")
self._add_xml_nml_file(data_nml_path, "namelist_definition_air_comp.xml")
self._add_xml_nml_file(data_nml_path, "namelist_definition_ref_pres.xml")

#----------------------------------------------------
Expand Down
122 changes: 101 additions & 21 deletions src/control/cam_comp.F90
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,28 @@ module cam_comp
!
!-----------------------------------------------------------------------

use shr_kind_mod, only: r8 => SHR_KIND_R8
use shr_kind_mod, only: cl=>SHR_KIND_CL, cs=>SHR_KIND_CS
use shr_sys_mod, only: shr_sys_flush

use spmd_utils, only: masterproc, mpicom
use cam_control_mod, only: cam_ctrl_init, cam_ctrl_set_orbit, cam_ctrl_set_physics_type
use cam_control_mod, only: caseid, ctitle
use runtime_opts, only: read_namelist
use runtime_obj, only: cam_runtime_opts
use time_manager, only: timemgr_init, get_step_size
use time_manager, only: get_nstep, is_first_step, is_first_restart_step

use camsrfexch, only: cam_out_t, cam_in_t
use physics_types, only: phys_state, phys_tend
use dyn_comp, only: dyn_import_t, dyn_export_t

use perf_mod, only: t_barrierf, t_startf, t_stopf
use cam_logfile, only: iulog
use cam_abortutils, only: endrun
use shr_kind_mod, only: r8 => SHR_KIND_R8
use shr_kind_mod, only: cl=>SHR_KIND_CL, cs=>SHR_KIND_CS, cx=>SHR_KIND_CX
use shr_sys_mod, only: shr_sys_flush

use spmd_utils, only: masterproc, mpicom
use cam_control_mod, only: cam_ctrl_init, cam_ctrl_set_orbit
use cam_control_mod, only: cam_ctrl_set_physics_type
use cam_control_mod, only: caseid, ctitle
use runtime_opts, only: read_namelist
use runtime_obj, only: cam_runtime_opts
use time_manager, only: timemgr_init, get_step_size
use time_manager, only: get_nstep
use time_manager, only: is_first_step, is_first_restart_step

use camsrfexch, only: cam_out_t, cam_in_t
use physics_types, only: phys_state, phys_tend
use dyn_comp, only: dyn_import_t, dyn_export_t

use perf_mod, only: t_barrierf, t_startf, t_stopf
use cam_logfile, only: iulog
use cam_abortutils, only: endrun
use ccpp_constituent_prop_mod, only: ccpp_constituent_properties_t

implicit none
private
Expand All @@ -47,6 +50,16 @@ module cam_comp

logical :: BFB_CAM_SCAM_IOP = .false.

! Currently, the host (CAM) only adds water vapor (specific_humidity)
! as a constituent.
! Does this need to be a configurable variable?
integer, parameter :: num_host_advected = 1
type(ccpp_constituent_properties_t), target :: host_constituents(num_host_advected)

! Private interface (here to avoid circular dependency)
private :: cam_register_constituents


!-----------------------------------------------------------------------
CONTAINS
!-----------------------------------------------------------------------
Expand Down Expand Up @@ -80,6 +93,9 @@ subroutine cam_init(caseid, ctitle, model_doi_url, &
! use history_defaults, only: initialize_iop_history
use stepon, only: stepon_init
use air_composition, only: air_composition_init
use cam_ccpp_cap, only: cam_ccpp_initialize_constituents
use physics_grid, only: columns_on_task
use vert_coord, only: pver

! Arguments
character(len=cl), intent(in) :: caseid ! case ID
Expand Down Expand Up @@ -122,6 +138,8 @@ subroutine cam_init(caseid, ctitle, model_doi_url, &

! Local variables
character(len=cs) :: filein ! Input namelist filename
integer :: errflg
character(len=cx) :: errmsg
!-----------------------------------------------------------------------

call init_pio_subsystem()
Expand Down Expand Up @@ -153,12 +171,20 @@ subroutine cam_init(caseid, ctitle, model_doi_url, &
! Open initial or restart file, and topo file if specified.
call cam_initfiles_open()

! Initialize model grids and decompositions
call model_grid_init()
! Initialize constituent information
! This will set the total number of constituents and the
! number of advected constituents.
call cam_register_constituents(cam_runtime_opts)

! Initialize composition-dependent constants:
call air_composition_init()

! Initialize model grids and decompositions
call model_grid_init()

! Initialize constituent data
call cam_ccpp_initialize_constituents(columns_on_task, pver, errflg, errmsg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've gone back and forth on this one, but thought I'd put it out there for discussion. I am uncomfortable about requiring host models to add a call for a subroutine which does not exist, but is auto-generated. Is there a way that this call could be hidden away in auto-generated CCPP code?

Up until this PR, we had the 5 phases as the only special code needed to interface with the CCPP. With this PR, we are adding "register", "initialize" and "number" for constituents. Each have their own API and there is no code to look at until the CCPP is run. Obviously this will need to be extensively documented, but I am wondering if the calls be hidden away somehow?

This code to interface with the CCPP is not adhering to the "just add the 5 phase calls and supply metadata for your scheme and it should work".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would making another routine (cam_initialize_constituents?) at the end of this module to "hide" this away be sufficient?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've gone back and forth on this one, but thought I'd put it out there for discussion. I am uncomfortable about requiring host models to add a call for a subroutine which does not exist, but is auto-generated. Is there a way that this call could be hidden away in auto-generated CCPP code?

This has been in the system for years. It was discussed and approved way back when the NRL folks met with the CCPP Framework committee and pointed out the lack of support for communicating with the dycore. It is not a new interface. It is not one of the 'required 5' phase calls (so it is technically optional) but models need it to function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gold2718 - I don't remember this, but that doesn't mean anything. So you are saying the Dom and Grant approved this, correct, that there are three new optional interfaces to the CCPP?

Thinking about this further, I'd like to propose another possible solution. That would be instead to add an optional "6th" phase that would be analogous to our "register" phase in CAM. I do remember early on you and I discussed this phase and you said we'd implement that phase if needed. @peverwhee, @gold2718 and @nusbaume, would this be a way to hide these three object interfaces?

I'm concerned that if we end up adding additional CCPP-blessed objects in the future that the number of interfaces will proliferate. To me that makes the threshold of host models adopting CCPP higher (with every additional interface that needs to be potentially added).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about this further, I'd like to propose another possible solution. That would be instead to add an optional "6th" phase that would be analogous to our "register" phase in CAM.

I think the problem with this approach to initializing advected consitituents is that any interface that hides what is going on has to be compatible with how all the models (e.g., CAM, UFS, NEPTUNE) bring up their dycore structures. Lumping everything together might look 'simpler' from a 'number of CCPP interfaces' perspective but it will lead to more complex implementation in the host models (including CAMDEN) which will then have to parse out the data returned by the new pre-packaged interface and add the data to their internal structures. We can consider this if you get buy-in from NOAA and NRL (more NRL since the UFS still has their advected constituents listed in a separate text file that has to magically work with the CCPP fields at runtime).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

opened #215 for continued discussion

Copy link
Collaborator

Choose a reason for hiding this comment

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

Intentionally not resolving this, so the discussion is readily viewable when #215 is worked upon.


! Initialize ghg surface values before default initial distributions
! are set in dyn_init
!!XXgoldyXX: This needs to be converted to CCPP and the issue of
Expand Down Expand Up @@ -461,6 +487,60 @@ subroutine cam_final(cam_out, cam_in)

end subroutine cam_final

!-----------------------------------------------------------------------

subroutine cam_register_constituents(cam_runtime_opts)
! Call the CCPP interface to register all constituents for the
! physics suite being invoked during this run.
use cam_abortutils, only: endrun
use runtime_obj, only: runtime_options
use cam_constituents, only: cam_constituents_init
use ccpp_constituent_prop_mod, only: ccpp_constituent_prop_ptr_t
use cam_ccpp_cap, only: cam_ccpp_register_constituents
use cam_ccpp_cap, only: cam_ccpp_number_constituents
use cam_ccpp_cap, only: cam_model_const_properties
use cam_ccpp_cap, only: cam_const_get_index

! Dummy arguments
type(runtime_options), intent(in) :: cam_runtime_opts
! Local variables
integer :: index
integer :: num_advect
integer, allocatable :: ind_water_spec(:)
integer :: errflg
character(len=cx) :: errmsg
type(ccpp_constituent_prop_ptr_t), pointer :: const_props(:)
character(len=*), parameter :: subname = 'cam_register_constituents: '

! Register the constituents to find out what needs advecting
call host_constituents(1)%initialize(std_name="specific_humidity", &
long_name="Specific humidity", units="kg kg-1", &
vertical_dim="vertical_layer_dimension", advected=.true., &
errcode=errflg, errmsg=errmsg)
if (errflg /= 0) then
call endrun(subname//trim(errmsg), file=__FILE__, line=__LINE__)
end if
call cam_ccpp_register_constituents(cam_runtime_opts%suite_as_list(), &
host_constituents, errcode=errflg, errmsg=errmsg)

if (errflg /= 0) then
call endrun(subname//trim(errmsg), file=__FILE__, line=__LINE__)
end if
call cam_ccpp_number_constituents(num_advect, advected=.true., &
errcode=errflg, errmsg=errmsg)

if (errflg /= 0) then
call endrun(subname//trim(errmsg), file=__FILE__, line=__LINE__)
end if

! Grab a pointer to the constituent array
const_props => cam_model_const_properties()

! Finally, initialize the constituents module
call cam_constituents_init(const_props, num_advect)

end subroutine cam_register_constituents

!-----------------------------------------------------------------------

end module cam_comp
15 changes: 11 additions & 4 deletions src/control/cam_logfile.F90
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@ module cam_logfile
!-----------------------------------------------------------------------
! Public data ----------------------------------------------------------
!-----------------------------------------------------------------------
integer, public, protected :: iulog = 6
integer, public, parameter :: DEBUGOUT_NONE = 0
integer, public, parameter :: DEBUGOUT_INFO = 1
integer, public, parameter :: DEBUGOUT_VERBOSE = 2
integer, public, parameter :: DEBUGOUT_DEBUG = 3
integer, public, protected :: debug_output = DEBUGOUT_NONE
!> \section arg_table_cam_logfile Argument Table
!! \htmlinclude cam_logfile.html
integer, public, protected :: iulog = 6
logical, public, protected :: log_output = .false.

!-----------------------------------------------------------------------
! Private data ---------------------------------------------------------
Expand Down Expand Up @@ -68,9 +71,9 @@ subroutine cam_set_log_unit(unit_num)
end subroutine cam_set_log_unit

subroutine cam_logfile_readnl(nlfile)
use shr_nl_mod, only: find_group_name => shr_nl_find_group_name
use spmd_utils, only: mpicom, masterprocid, masterproc
use mpi, only: mpi_integer
use mpi, only: mpi_integer
use shr_nl_mod, only: find_group_name => shr_nl_find_group_name
use spmd_utils, only: mpicom, masterprocid, masterproc

! nlfile: filepath for file containing namelist input
character(len=*), intent(in) :: nlfile
Expand All @@ -84,6 +87,10 @@ subroutine cam_logfile_readnl(nlfile)
namelist /cam_logfile_nl/ debug_output
!------------------------------------------------------------------------

! Since cam_set_log_unit is called before spmd_init is called,
! set log_output flag here
log_output = masterproc

if (masterproc) then
open(newunit=unitn, file=trim(nlfile), status='old')
call find_group_name(unitn, 'cam_logfile_nl', status=ierr)
Expand Down
19 changes: 19 additions & 0 deletions src/control/cam_logfile.meta
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[ccpp-table-properties]
name = cam_logfile
type = module

[ccpp-arg-table]
name = cam_logfile
type = module
[ iulog ]
standard_name = log_output_unit
units = 1
type = integer
dimensions = ()
protected = True
[ log_output ]
standard_name = do_log_output
units = flag
type = logical
dimensions = ()
protected = True
2 changes: 0 additions & 2 deletions src/control/camsrfexch.F90
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ module camsrfexch
!-----------------------------------------------------------------------

use shr_kind_mod, only: r8 => shr_kind_r8, r4 => shr_kind_r4
use constituents, only: pcnst
use shr_infnan_mod, only: posinf => shr_infnan_posinf, assignment(=)
use cam_abortutils, only: endrun
use string_utils, only: to_str
Expand Down Expand Up @@ -155,7 +154,6 @@ subroutine cam_export(state, cam_out)
use physics_types, only: physics_state
use vert_coord, only: pver
use physconst, only: rair, mwdry, mwco2, gravit
use constituents, only: pcnst

! Input arguments
type(physics_state), intent(in) :: state
Expand Down
Loading