sima0_16_001: Make constituent names case-insensitive in IC file read path, consistent with the CCPP framework; update framework tag by jimmielin · Pull Request #518 · ESCOMP/CAM-SIMA

jimmielin · 2026-07-06T17:14:35Z

Tag name (required for release branches):
Originator(s): @jimmielin
AI tools used (if applicable; please also add the "AI-generated code" label to the PR):
What: see below
How: claude-fable:5 identified and drafted the original case-insensitive fix; claude-opus:4.8 refined the fix

Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):

Case-insensitive constituent ic_file_input_name matching. Standard names are case-insensitive and capgen lowercases scheme-side names (e.g. co2), but the IC read code compared them case-sensitively so a registry constituent with authored case (CO2, O3) or an uppercase snapshot field read nothing (could see this failure in atm.log... it falls back to the argument name). All constituent-name comparisons are now case-insensitive.
All-case cnst_/pbuf_ fallback. For constituents not enumerated in the registry, the read/check fallback now tries {std_name, cnst_, pbuf_} × {as-is, UPPER, lower} so a lowercase runtime name (co2) still matches an uppercase snapshot field (CO2, cnst_CO2). netCDF variable names are case-sensitive. CAM has uppercase for most chemistry and physics fields and lowercase for aerosols (e.g., num_a1)
Suppress local-name rows for non-registry constituents. gather_ccpp_req_vars no longer adds constituents to the required host-variable list, which previously gave them a phys_var_stdnames row keyed to the scheme's local Fortran name (e.g. co2mmr) that preempted the cnst_/pbuf_ fallback and left the constituent at its default.

Describe any changes made to build system: N/A

Describe any changes made to the namelist: N/A

List any changes to the defaults for the input datasets (e.g. boundary datasets):

List all files eliminated and why:

List all files added and what they do:

List all existing files that have been modified, and describe the changes:
(Helpful git command: git diff --name-status development...<your_branch_name>)

M  src/data/cam_var_init_marks.inc
  - Case-insensitive mark_as_*/is_* standard-name lookups.

M  src/data/write_init_files.py
  - Generated phys_read_data/phys_check_data now match constituent standard
    names case-insensitively and try all-case cnst_/pbuf_ candidates in the
    not-in-registry fallback.
  - gather_ccpp_req_vars no longer adds constituents to in_vars so the fallback
    can actually be reached for meta-declared constituents.

M  src/physics/utils/physics_data.F90
  - Case-insensitive standard-name matching in find_input_name_idx and the
    constituent-dimensioned shortmap loop.
  - New helper constituent_dim_file_var_names builds <base>_<name> in the
    as-registered / all-upper / all-lower spellings for constituent-dimensioned
    reads (e.g. cam_in_cflx_CO2 vs cam_in_cflx_co2).

M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_4D.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_bvd.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_cnst.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_constituent_dim.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_ddt.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_ddt2.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_ddt_array.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_host_var.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_initial_value.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_mf.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_no_horiz.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_noreq.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_param.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_protect.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_scalar.F90
M       test/unit/python/sample_files/write_init_files/phys_vars_init_check_simple.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_4D.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_bvd.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_cnst.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_constituent_dim.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_ddt.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_ddt2.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_ddt_array.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_host_var.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_initial_value.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_mf.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_no_horiz.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_noreq.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_param.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_protect.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_scalar.F90
M       test/unit/python/sample_files/write_init_files/physics_inputs_simple.F90
  - auto-generated file updates due to write_init_files.py update

If there are new failures (compared to the test/existing-test-failures.txt file),
have them OK'd by the gatekeeper, note them here, and add them to the file.
If there are baseline differences, include the test and the reason for the
diff. What is the nature of the change? Roundoff?

derecho/intel/aux_sima:
SMS_Ln9.ne3pg3_ne3pg3_mg37.FKESSLER.derecho_intel.cam-outfrq_se_cslam_multitape (Overall: NLFAIL) details:
FAIL SMS_Ln9.ne3pg3_ne3pg3_mg37.FKESSLER.derecho_intel.cam-outfrq_se_cslam_multitape NLCOMP
Pre-existing failure

derecho/gnu/aux_sima:
SMS_Ln9.ne3pg3_ne3pg3_mg37.FADIAB.derecho_gnu.cam-outfrq_se_cslam (Overall: DIFF) details:
FAIL SMS_Ln9.ne3pg3_ne3pg3_mg37.FADIAB.derecho_gnu.cam-outfrq_se_cslam BASELINE /glade/campaign/cesm/community/amwg/sima_baselines/latest_gnu: ERROR BFAIL some baseline files were missing
Suspect test

derecho/nvhpc/aux_sima (test is run via Github workflow. Only run the test manually if we need to save new baselines): All PASS

If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced:

CAM-SIMA date used for the baseline comparison tests if different than latest:

…allback Cherry-picks the case-insensitive standard-name matching from e4327dd and extends the constituent ic-read fallback to try all-upper and all-lower case spellings. Standard names are case-insensitive -- capgen lowercases scheme-side names (e.g. 'co2') while registry declarations keep their authored case ('CO2', 'O3', ...) -- but the ic-read glue compared them case-sensitively, and the not-in-registry snapshot fallback built candidate netCDF names only in the runtime (lowercase) case. netCDF variable names are case-sensitive, so a registry constituent declared with authored case, or an uppercase snapshot field ('CO2', 'cnst_CO2'), silently failed to match and fell back to zeros. - physics_data.F90: find_input_name_idx and the constituent-dimensioned shortmap loop compare case-insensitively; a new helper builds constituent-dimensioned file names in as-registered/upper/lower spellings. - write_init_files.py: generated phys_read_data/phys_check_data compare case-insensitively, and the not-in-registry fallback now tries {std_name, cnst_<>, pbuf_<>} x {as-is, upper, lower}. - cam_var_init_marks.inc: mark_as_*/is_* lookups compare case-insensitively. Preserves 0d6d482's std_name_len+5 sizing (a raw cherry-pick of e4327dd would have regressed it). Regenerated write_init_files sample fixtures; write_init_files (16/16) and registry (29/29) python unit tests pass.

gather_ccpp_req_vars added any constituent not listed in registry.xml to the required host-variable list, which gave it a phys_var_stdnames row keyed to the scheme's local Fortran variable name (e.g. 'co2mmr' for CO2 in modal_aero_setsox). At runtime that phantom row matched (const_input_idx>0), so the constituent was read by its local name -- which never matches a CAM snapshot field (constituents are written as <CNST_NAME>/cnst_<CNST_NAME>, uppercase, and netCDF names are case-sensitive) -- pre-empting the cnst_/pbuf_ standard-name fallback and silently leaving the constituent at its default (the setsox xco2=0 symptom). Only advected constituents are suppressed from in_vars; constituent-related variables that are not advected (e.g. tendency variables like tendency_of_water_vapor_mixing_ratio_wrt_moist_air_and_condensed_water) are still added as regular host variables -- they are read via the host variable path, not the constituent loop. The now-unused registry_constituents argument to gather_ccpp_req_vars is dropped and write_ic_arrays' registry skip is restored to its original form. Sample fixtures unchanged; write_init_files (16/16) and registry (29/29) pass.

peverwhee

One question.

Also: I'm hopeful that once we get rid of build-time constituents and everything is runtime, we'll be able to use the constituent diag_name to ease some of this logic.

peverwhee

thanks @jimmielin !

The new framework tag is available for you: sima_2026-07-15

Co-authored-by: Courtney Peverley <courtneyp@ucar.edu>

jimmielin · 2026-07-15T18:01:39Z

Thanks @peverwhee! Regression tests started! Wish me luck!

jimmielin · 2026-07-16T14:11:10Z

It looks like the regression tests passed, but CIRRUS won't work so I will end up running nvhpc instead.

nusbaume

Thanks @jimmielin! I have a few requests, mostly related to what looks like the AI being overly-verbose, but nothing that is strictly required or needs a re-review.

nusbaume · 2026-07-16T03:12:02Z

+                    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:


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!

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'):

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

nusbaume · 2026-07-16T03:22:27Z

+    # 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.


I might get rid of this comment block, as the text in the generated comments explains it well enough, at least in my opinion.

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)

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!

nusbaume · 2026-07-16T03:36:37Z

+    # 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.


I would remove this comment-block as well.

Please see question above!

nusbaume · 2026-07-16T03:56:46Z

+
+      character(len=*), intent(in) :: base_name
+      character(len=*), intent(in) :: cname
+      character(len=256)           :: var_names(3)


Since this is SIMA-specific code we can use the shared CESM character lengths:

Suggested change

character(len=256) :: var_names(3)

use shr_kind_mod, only: cl=>shr_kind_cl

...

character(len=cl) :: var_names(3)

jimmielin · 2026-07-18T15:16:12Z

I ran the regression tests last night and it looks like trcdata_bam is failing because it uses the legacy ccpp_const_utils shim (which is case sensitive). I will change that to ccpp_scheme_utils, as well as another bug fix needed, which will have to be in atmos_phys. This will be a Monday problem...

jimmielin · 2026-07-18T23:06:32Z

Needs the fixes from ESCOMP/atmospheric_physics#424 then this will be ready to merge - bfb.

@jimmielin

… fully use diagnostic names in sima_state_diagnostics (#424) Tag name (The PR title should also include the tag name): Originator(s): @jimmielin Description (include issue title and the keyword ['closes', 'fixes', 'resolves'] and issue number): - Change prescribed chemistry/aerosol schemes to use ccpp_scheme_utils so they are case-insensitive - companion PR to ESCOMP/CAM-SIMA#518 - #397 (in 979c6bc) changed `sima_state_diagnostics` to use `%diagnostic_name` to add field but not the write loop, so if the two cases mismatched (due to the case-insensitivity fixes they now do) the `history_out_field` would be mismatched and would write uninitialized heap memory into the netCDF file rather than the variable. List all namelist files that were added or changed: List all files eliminated and why: List all files added and what they do: List all existing files that have been modified, and describe the changes: (Helpful git command: `git diff --name-status main...<your_branch_name>`) ``` M schemes/chemistry/prescribed_aerosols.F90 M schemes/chemistry/prescribed_aerosols.meta M schemes/chemistry/prescribed_ozone.F90 M schemes/chemistry/prescribed_ozone.meta - change to use ccpp_scheme_utils. M schemes/sima_diagnostics/sima_state_diagnostics.F90 - change to use diagnostic_name in history_out_field. ``` List all automated tests that failed, as well as an explanation for why they weren't fixed: Is this an answer-changing PR? If so, is it a new physics package, algorithm change, tuning change, etc? If yes to the above question, describe how this code was validated with the new/modified features: --------- Co-authored-by: Jesse Nusbaumer <nusbaume@ucar.edu>

jimmielin · 2026-07-22T14:21:07Z

This is not answer-changing but the GNU test ran to completion, so I am giving it a tag nubmer to archive the baseline.

. ESCOMP#518 stopped gather_ccpp_req_vars adding constituents to the required host-variable list, which drops 'band_number' from the bad-vertical-dimension sample (phys_var_num 3 -> 2, ic_name_len 7 -> 5), but the committed sample file was not regenerated. test_write_init_files::test_bad_vertical_dimension fails on development as a result. Regenerated output is byte-identical to what development's own write_init_files.py produces, so this applies to development standalone.

The constituent read/check paths are now generated by development's ESCOMP#518 logic (case-insensitive standard-name matching plus the all-case cnst_/pbuf_ candidate list) rather than the superseded branch-local version that was dropped in this rebase.

jimmielin temporarily deployed to CI-tests-on-CIRRUS July 6, 2026 17:14 — with GitHub Actions Inactive

jimmielin force-pushed the hplin/case_insensitive_constituents_bug branch from 3cd5c84 to 4ec5892 Compare July 7, 2026 04:36

jimmielin temporarily deployed to CI-tests-on-CIRRUS July 7, 2026 04:36 — with GitHub Actions Inactive

jimmielin marked this pull request as ready for review July 7, 2026 15:29

jimmielin requested a review from peverwhee July 7, 2026 15:29

peverwhee reviewed Jul 9, 2026

View reviewed changes

Comment thread src/data/write_init_files.py Outdated

Add protected = True to dimension variables

5f84e58

jimmielin had a problem deploying to CI-tests-on-CIRRUS July 11, 2026 20:44 — with GitHub Actions Error

Merge branch 'development' into hplin/case_insensitive_constituents_bug

8f099a5

jimmielin had a problem deploying to CI-tests-on-CIRRUS July 13, 2026 16:45 — with GitHub Actions Failure

peverwhee approved these changes Jul 15, 2026

View reviewed changes

peverwhee requested a review from nusbaume July 15, 2026 17:58

Update CCPP framework tag to sima_2026-07-15.

f155f64

Co-authored-by: Courtney Peverley <courtneyp@ucar.edu>

jimmielin temporarily deployed to CI-tests-on-CIRRUS July 15, 2026 18:01 — with GitHub Actions Inactive

Fix Python unit tests.

0d69c6a

jimmielin had a problem deploying to CI-tests-on-CIRRUS July 15, 2026 18:15 — with GitHub Actions Failure

jimmielin had a problem deploying to CI-tests-on-CIRRUS July 16, 2026 14:09 — with GitHub Actions Failure

jimmielin changed the title ~~Make constituent names case-insensitive in IC file read path, consistent with the CCPP framework~~ Make constituent names case-insensitive in IC file read path, consistent with the CCPP framework; update framework tag Jul 16, 2026

jimmielin had a problem deploying to CI-tests-on-CIRRUS July 16, 2026 17:31 — with GitHub Actions Failure

jimmielin had a problem deploying to CI-tests-on-CIRRUS July 16, 2026 17:34 — with GitHub Actions Failure

jimmielin temporarily deployed to CI-tests-on-CIRRUS July 16, 2026 18:15 — with GitHub Actions Inactive

nusbaume approved these changes Jul 16, 2026

View reviewed changes

peverwhee mentioned this pull request Jul 16, 2026

Cleanup/manual review of feature/cagen-v1 part 1 + 2 minor improvements NCAR/ccpp-framework#764

Merged

3 tasks

jimmielin added 2 commits July 17, 2026 22:34

Merge branch 'development' into hplin/case_insensitive_constituents_bug

4cc3890

Address comments from code review.

1e509e9

jimmielin temporarily deployed to CI-tests-on-CIRRUS July 18, 2026 02:36 — with GitHub Actions Inactive

jimmielin added AI-generated code BFB bit for bit tag externals externals updating issue or PR labels Jul 18, 2026

jimmielin mentioned this pull request Jul 18, 2026

Change prescribed chemistry/aerosol schemes to use ccpp_scheme_utils; fully use diagnostic names in sima_state_diagnostics ESCOMP/atmospheric_physics#424

Merged

Update atmos_phys to latest hash

43cfa38

jimmielin deployed to CI-tests-on-CIRRUS July 21, 2026 23:48 — with GitHub Actions Active

jimmielin changed the title ~~Make constituent names case-insensitive in IC file read path, consistent with the CCPP framework; update framework tag~~ sima0_16_001: Make constituent names case-insensitive in IC file read path, consistent with the CCPP framework; update framework tag Jul 22, 2026

jimmielin merged commit 1ca6e5b into ESCOMP:development Jul 22, 2026
23 checks passed

-      character(len=256)           :: var_names(3)
+      use shr_kind_mod, only: cl=>shr_kind_cl
+      ...
+      character(len=cl)           :: var_names(3)

Uh oh!

Conversation

jimmielin commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peverwhee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peverwhee left a comment

Choose a reason for hiding this comment

Uh oh!

jimmielin commented Jul 15, 2026

Uh oh!

jimmielin commented Jul 16, 2026

Uh oh!

nusbaume left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimmielin commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jimmielin commented Jul 18, 2026

Uh oh!

jimmielin commented Jul 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jimmielin commented Jul 6, 2026 •

edited

Loading

jimmielin commented Jul 18, 2026 •

edited

Loading