Skip to content

sima0_16_001: Make constituent names case-insensitive in IC file read path, consistent with the CCPP framework; update framework tag #518

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.

Sign up for GitHub

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

Merged
jimmielin merged 9 commits into from
Jul 22, 2026
Merged

sima0_16_001: Make constituent names case-insensitive in IC file read path, consistent with the CCPP framework; update framework tag #518

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
90895b8
Case-insensitive constituent ic-name matching; all-case cnst_/pbuf_ f…
Jul 6, 2026
4ec5892
Suppress phantom local-name rows for non-registry constituents
Jul 6, 2026
5f84e58
Add protected = True to dimension variables
Jul 11, 2026
8f099a5
Merge branch 'development' into hplin/case_insensitive_constituents_bug
Jul 13, 2026
f155f64
Update CCPP framework tag to sima_2026-07-15.
Jul 15, 2026
0d69c6a
Fix Python unit tests.
Jul 15, 2026
4cc3890
Merge branch 'development' into hplin/case_insensitive_constituents_bug
Jul 18, 2026
1e509e9
Address comments from code review.
Jul 18, 2026
43cfa38
Update atmos_phys to latest hash
Jul 21, 2026
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 .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[submodule "ccpp-framework"]
path = ccpp_framework
url = https://github.com/NCAR/ccpp-framework
fxtag = sima_2026-04-13
fxtag = sima_2026-07-15
fxrequired = AlwaysRequired
fxDONOTUSEurl = https://github.com/NCAR/ccpp-framework
[submodule "history"]
Expand Down
2 changes: 1 addition & 1 deletion ccpp_framework
Open in desktop
Submodule ccpp_framework updated 115 files
7 changes: 6 additions & 1 deletion cime_config/create_readnl_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,12 +404,17 @@ def missing(self):
def write_metadata_entry(self, file):
"""Write a metadata entry for this NLVar object to <file>."""
if self.array_len:
# Write unique array dimension variable(s)
# Write unique array dimension variable(s). These are declared
# as Fortran parameters (see write_dimension_decls), so mark them
# protected: schemes only use them as dimensions, and the host
# provides their values, so they must not be treated as fields to
# read from the initial-conditions file.
for aname in self.__array_names:
file.write(f"[ {aname} ]\n")
file.write(f" standard_name = {aname}\n")
file.write(" type = integer | units = 1\n")
file.write(" dimensions = ()\n")
file.write(" protected = True\n")
# end for
# end if (no else)
file.write(f"[ {self.var_name} ]\n")
Expand Down
15 changes: 10 additions & 5 deletions src/data/cam_var_init_marks.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
! which means any initialization check should
! now return True.

use string_utils, only: to_lower

! Dummy argument
character(len=*), intent(in) :: varname !Variable name being marked

Expand All @@ -13,7 +15,7 @@

! Search for <varname> in the standard name array:
do stdnam_idx = 1, phys_var_num
if (trim(phys_var_stdnames(stdnam_idx)) == trim(varname)) then
if (to_lower(trim(phys_var_stdnames(stdnam_idx))) == to_lower(trim(varname))) then
! Only set to INITIALIZED if state is UNINITIALIZED
if (initialized_vars(stdnam_idx) < PARAM) then
initialized_vars(stdnam_idx) = INITIALIZED
Expand All @@ -33,6 +35,7 @@
! initialized_vars array

use cam_abortutils, only: endrun
use string_utils, only: to_lower

! Dummy argument
character(len=*), intent(in) :: varname ! Variable name being marked
Expand All @@ -45,7 +48,7 @@
found_var = .false.
! Set variable to READ_FROM_FILE:
do stdnam_idx = 1, phys_var_num
if (trim(phys_var_stdnames(stdnam_idx)) == trim(varname)) then
if (to_lower(trim(phys_var_stdnames(stdnam_idx))) == to_lower(trim(varname))) then
! It is an error if the variable has already been set to PARAM
if (initialized_vars(stdnam_idx) == PARAM) then
call endrun("Variable '"//trim(varname)// &
Expand Down Expand Up @@ -73,6 +76,7 @@
! initialized according to the 'initialized_vars' array.

use cam_abortutils, only: endrun
use string_utils, only: to_lower

! Dummy argument
character(len=*), intent(in) :: varname ! Variable name being checked
Expand All @@ -87,7 +91,7 @@

! Check if variable is initialized (PARAM, INITIALIZED, or READ_FROM_FILE)
do stdnam_idx = 1, phys_var_num
if (trim(phys_var_stdnames(stdnam_idx)) == trim(varname)) then
if (to_lower(trim(phys_var_stdnames(stdnam_idx))) == to_lower(trim(varname))) then
is_initialized = (initialized_vars(stdnam_idx) > UNINITIALIZED)
found = .true.
exit ! Exit loop once variable has been found and checked
Expand All @@ -108,6 +112,7 @@
! file according to the 'initialized_vars' array.

use cam_abortutils, only: endrun
use string_utils, only: to_lower

! Dummy arguments
character(len=*), intent(in) :: varname ! Variable name being checked
Expand All @@ -125,7 +130,7 @@

! Return .true. if the variable's status is READ_FROM_FILE:
do stdnam_idx = 1, phys_var_num
if (trim(phys_var_stdnames(stdnam_idx)) == trim(varname)) then
if (to_lower(trim(phys_var_stdnames(stdnam_idx))) == to_lower(trim(varname))) then
is_read = (initialized_vars(stdnam_idx) == READ_FROM_FILE)
! Mark as found:
found = .true.
Expand All @@ -136,7 +141,7 @@
if (.not. found) then
! Check to see if this is an internally-protected variable
do stdnam_idx = 1, phys_const_num
if (trim(phys_const_stdnames(stdnam_idx)) == trim(varname)) then
if (to_lower(trim(phys_const_stdnames(stdnam_idx))) == to_lower(trim(varname))) then
found = .true.
exit ! Exit loop once variable has been found
end if
Expand Down
85 changes: 54 additions & 31 deletions src/data/write_init_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def write_init_files(cap_database, ic_names, registry_constituents, vars_init_va

# Gather all the host model variables that are required by
# any of the compiled CCPP physics suites.
in_vars, out_vars, constituent_set, retmsg = gather_ccpp_req_vars(cap_database, registry_constituents)
in_vars, out_vars, constituent_set, retmsg = gather_ccpp_req_vars(cap_database)

# Quit now if there are missing variables
if retmsg:
Expand Down Expand Up @@ -306,7 +306,7 @@ def _find_and_add_host_variable(stdname, host_dict, var_dict):
return missing_vars

##############################################################################
def gather_ccpp_req_vars(cap_database, registry_constituents):
def gather_ccpp_req_vars(cap_database):
"""
Generate a list of host-model and constituent variables
required by the CCPP physics suites potentially being used
Expand Down Expand Up @@ -340,10 +340,21 @@ def gather_ccpp_req_vars(cap_database, registry_constituents):
(stdname not in in_vars) and
(stdname not in _EXCLUDED_STDNAMES)):
if is_const:
#Add variable to constituent set:
constituent_vars.add(stdname)
#Add variable to required variable list if it's not a registry constituent
if stdname not in registry_constituents:
if cvar.get_prop_value('advected'):
# Advected constituents are allocated/registered by the
# constituents object and read via the runtime
# constituent loop (registry ic_file_input_names or
# the cnst_/pbuf_ fallback). Do NOT add them to the
# host-variable list: that gives them a
# phys_var_stdnames row keyed to the scheme's local
# name, which never matches a CAM snapshot field and
# would pre-empt the fallback.
pass
else:

@nusbaume nusbaume Jul 16, 2026

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 feels like a situation where the AI is over-explaining something (i.e. most of time a human wouldn't write a paragraph explaining the no-op path of a conditional). Instead I might just change the conditional to its inverse and remove this pass section, especially since the else comment seems to explain it well enough.

Suggested change
if cvar.get_prop_value('advected'):
# Advected constituents are allocated/registered by the
# constituents object and read via the runtime
# constituent loop (registry ic_file_input_names or
# the cnst_/pbuf_ fallback). Do NOT add them to the
# host-variable list: that gives them a
# phys_var_stdnames row keyed to the scheme's local
# name, which never matches a CAM snapshot field and
# would pre-empt the fallback.
pass
else:
if not cvar.get_prop_value('advected'):

Of course if you disagree just let me know, as I could certainly be convinced otherwise!

@jimmielin jimmielin Jul 16, 2026

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.

Thanks! I agree this is overly verbose but I think we could document the if not clause with a reason. Would this be better?

                    # Do not add advected constituents to the host variable
                    # list so they are not included in phys_var_stdnames and
                    # can be read in the constituent path:
                    if not cvar.get_prop_value('advected'):

@nusbaume nusbaume Jul 18, 2026

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.

Yeah that comment block for the if not clause looks good to me, so feel free to add it. Thanks!

# Constituent-related but not advected (e.g. tendency
# variables): these are read as regular host variables,
# not through the constituent loop.
in_vars[stdname] = cvar
# end if
Comment thread
peverwhee marked this conversation as resolved.
Outdated
else:
Expand Down Expand Up @@ -974,6 +985,7 @@ def write_phys_read_subroutine(outfile, host_dict, host_vars, host_imports,
"cam_constituents_array",
"cam_model_const_properties"]],
["ccpp_kinds", ["kind_phys"]],
["string_utils", ["to_lower", "to_upper"]],
[phys_check_fname_str, ["phys_var_num", "phys_var_stdnames",
"input_var_names", "std_name_len",
"is_initialized"]],
Expand Down Expand Up @@ -1180,26 +1192,31 @@ def write_phys_read_subroutine(outfile, host_dict, host_vars, host_imports,
outfile.write("var_found = .false.", 3)
outfile.comment("Check if constituent standard name in registered SIMA standard names list:", 3)
outfile.write("call const_props(constituent_idx)%standard_name(std_name)", 3)
outfile.write("if(any(phys_var_stdnames == trim(std_name))) then", 3)
outfile.comment("Find array index to extract correct input names", 3)
outfile.comment("(case-insensitive: see find_input_name_idx):", 3)
outfile.write("const_input_idx = -1", 3)
outfile.write("do n=1, phys_var_num", 3)
outfile.write("if(to_lower(trim(phys_var_stdnames(n))) == to_lower(trim(std_name))) then", 4)
outfile.write("const_input_idx = n", 5)
outfile.write("exit", 5)
outfile.write("end if", 4)
outfile.write("end do", 3)
outfile.write("if(const_input_idx > 0) then", 3)
outfile.comment("Don't read the variable in if it's already initialized", 4)
outfile.write("if (is_initialized(std_name)) then", 4)
outfile.write("cycle", 5)
outfile.write("end if", 4)
outfile.comment("Find array index to extract correct input names:", 4)
outfile.write("do n=1, phys_var_num", 4)
outfile.write("if(trim(phys_var_stdnames(n)) == trim(std_name)) then", 5)
outfile.write("const_input_idx = n", 6)
outfile.write("exit", 6)
outfile.write("end if", 5)
outfile.write("end do", 4)
outfile.write("call read_field(file, std_name, input_var_names(:,const_input_idx), 'lev', timestep, field_data_ptr(:,:,constituent_idx), mark_as_read=.false., error_on_not_found=.false., var_found=var_found)", 4)
outfile.write("else", 3)
outfile.comment("If not in standard names list, then attempt constituent name",4)
outfile.comment("and cnst_, pbuf_ prefixes used by CAM snapshots (advected, non-advected) as input names:",4)
# The <std_name>, cnst_<std_name>, pbuf_<std_name> prefix default fallbacks
# allows us to not enumerate all needed constituents from snapshots in the
# registry, yet allow reading their values from CAM snapshots.
outfile.write("call read_field(file, std_name, [character(len=std_name_len+5) :: std_name, 'cnst_'//trim(std_name), 'pbuf_'//trim(std_name)], 'lev', timestep, field_data_ptr(:,:,constituent_idx), mark_as_read=.false., error_on_not_found=.false., var_found=var_found)", 4)
outfile.comment("and cnst_, pbuf_ prefixes used by CAM snapshots (advected, non-advected) as input names.",4)
outfile.comment("Standard names are case-insensitive (capgen lowercases them) but netCDF names are not,",4)
outfile.comment("so also try the all-upper and all-lower case spellings of the constituent name:",4)
# The <std_name>, cnst_<std_name>, pbuf_<std_name> prefix default fallbacks --
# in the as-registered, all-upper, and all-lower case spellings -- allow us to
# not enumerate all needed constituents from snapshots in the registry, yet
# allow reading their values from CAM snapshots regardless of authored name case.

@nusbaume nusbaume Jul 16, 2026

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 might get rid of this comment block, as the text in the generated comments explains it well enough, at least in my opinion.

@jimmielin jimmielin Jul 16, 2026

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.

I also don't like this, so I am happy to remove. Would you think it's self-explanatory that some constituents in CAM are uppercase and some lowercase? I briefly considered changing this to something like

    # Try uppercase and lowercase variants for constituent names to match CAM
    # e.g., O3, NO2, NUMLIQ (most species) ... num_a1, num_a2 (aerosols)

@nusbaume nusbaume Jul 18, 2026

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 personally think it is self-explanatory, simply because having a system that tries the different case combinations is inherently more robust than a system that doesn't cover that space, regardless of what case(s) the constituents actually have.

That being said if you don't feel the same then I am happy to include the comment you are proposing (as it is still way shorter than what was originally there). Thanks!

outfile.write("call read_field(file, std_name, [character(len=std_name_len+5) :: std_name, 'cnst_'//trim(std_name), 'pbuf_'//trim(std_name), to_upper(std_name), 'cnst_'//trim(to_upper(std_name)), 'pbuf_'//trim(to_upper(std_name)), to_lower(std_name), 'cnst_'//trim(to_lower(std_name)), 'pbuf_'//trim(to_lower(std_name))], 'lev', timestep, field_data_ptr(:,:,constituent_idx), mark_as_read=.false., error_on_not_found=.false., var_found=var_found)", 4)
outfile.write("end if", 3)
outfile.write("if(.not. var_found) then", 3)
outfile.write("constituent_has_default = .false.", 4)
Expand Down Expand Up @@ -1306,6 +1323,7 @@ def write_phys_check_subroutine(outfile, host_dict, host_vars, host_imports,
"cam_model_const_properties"]],
["cam_constituents", ["const_get_index"]],
["ccpp_kinds", ["kind_phys"]],
["string_utils", ["to_lower", "to_upper"]],
["cam_logfile", ["iulog"]],
["spmd_utils", ["masterproc"]],
["phys_vars_init_check", ["is_read_from_file"]],
Expand Down Expand Up @@ -1492,25 +1510,30 @@ def write_phys_check_subroutine(outfile, host_dict, host_vars, host_imports,
outfile.write("do constituent_idx = 1, size(const_props)", 2)
outfile.comment("Check if constituent standard name in registered SIMA standard names list:", 3)
outfile.write("call const_props(constituent_idx)%standard_name(std_name)", 3)
outfile.write("if(any(phys_var_stdnames == std_name)) then", 3)
outfile.comment("Find array index to extract correct input names:", 4)
outfile.write("do n=1, phys_var_num", 4)
outfile.write("if(trim(phys_var_stdnames(n)) == trim(std_name)) then", 5)
outfile.write("const_input_idx = n", 6)
outfile.write("exit", 6)
outfile.write("end if", 5)
outfile.write("end do", 4)
outfile.comment("Find array index to extract correct input names", 3)
outfile.comment("(case-insensitive: see find_input_name_idx):", 3)
outfile.write("const_input_idx = -1", 3)
outfile.write("do n=1, phys_var_num", 3)
outfile.write("if(to_lower(trim(phys_var_stdnames(n))) == to_lower(trim(std_name))) then", 4)
outfile.write("const_input_idx = n", 5)
outfile.write("exit", 5)
outfile.write("end if", 4)
outfile.write("end do", 3)
outfile.write("if(const_input_idx > 0) then", 3)
outfile.write("call check_field(file, input_var_names(:,const_input_idx), 'lev', timestep, field_data_ptr(:,:,constituent_idx), std_name, min_difference, min_relative_value, is_first, diff_found)", 4)
outfile.write("if (diff_found) then", 4)
outfile.write("overall_diff_found = .true.", 5)
outfile.write("end if", 4)
outfile.write("else", 3)
outfile.comment("If not in standard names list, then attempt constituent name",4)
outfile.comment("and cnst_, pbuf_ prefixes used by CAM snapshots (advected, non-advected) as input names:",4)
# The <std_name>, cnst_<std_name>, pbuf_<std_name> prefix default fallbacks
# allows us to not enumerate all needed constituents from snapshots in the
# registry, yet allow reading their values from CAM snapshots.
outfile.write("call check_field(file, [character(len=std_name_len+5) :: std_name, 'cnst_'//trim(std_name), 'pbuf_'//trim(std_name)], 'lev', timestep, field_data_ptr(:,:,constituent_idx), std_name, min_difference, min_relative_value, is_first, diff_found)", 4)
outfile.comment("and cnst_, pbuf_ prefixes used by CAM snapshots (advected, non-advected) as input names.",4)
outfile.comment("Standard names are case-insensitive (capgen lowercases them) but netCDF names are not,",4)
outfile.comment("so also try the all-upper and all-lower case spellings of the constituent name:",4)
# The <std_name>, cnst_<std_name>, pbuf_<std_name> prefix default fallbacks --
# in the as-registered, all-upper, and all-lower case spellings -- allow us to
# not enumerate all needed constituents from snapshots in the registry, yet
# allow reading their values from CAM snapshots regardless of authored name case.

@nusbaume nusbaume Jul 16, 2026

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 would remove this comment-block as well.

@jimmielin jimmielin Jul 16, 2026

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.

Please see question above!

outfile.write("call check_field(file, [character(len=std_name_len+5) :: std_name, 'cnst_'//trim(std_name), 'pbuf_'//trim(std_name), to_upper(std_name), 'cnst_'//trim(to_upper(std_name)), 'pbuf_'//trim(to_upper(std_name)), to_lower(std_name), 'cnst_'//trim(to_lower(std_name)), 'pbuf_'//trim(to_lower(std_name))], 'lev', timestep, field_data_ptr(:,:,constituent_idx), std_name, min_difference, min_relative_value, is_first, diff_found)", 4)
outfile.write("if (diff_found) then", 4)
outfile.write("overall_diff_found = .true.", 5)
outfile.write("end if", 4)
Expand Down
Loading
Loading