Skip to content

Misc fixes identified by Claude (batch 2) - #554

Open
jimmielin wants to merge 4 commits into
ESCOMP:developmentfrom
jimmielin:hplin/audit_batch2
Open

jimmielin wants to merge 4 commits into
ESCOMP:developmentfrom
jimmielin:hplin/audit_batch2

Conversation

@jimmielin

@jimmielin jimmielin commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Tag name (required for release branches):
Originator(s):
AI tools used (if applicable; please also add the "AI-generated code" label to the PR):
What: Claude Code claude-opus:5, claude-fable:5
How: Opus workflow triaged the codebase and identified 129 potential issues; Fable triaged the issues and drafted the following 35 fixes.

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

This PR continues the work in #507 (batch 1) with further self-contained fixes that are small and tries to be 'obvious' to see when looking at the fixes independently.

  • ic_us_standard_atm.F90: nlev was never set for the T block, causing zero-size work arrays and uninitialized temperature
  • ic_us_standard_atm.F90: constituent loop compared the global constituent index against a findloc position and indexed Q with the global index
  • none/dyn_grid.F90: lon_index ignored the task's global column offset, assigning wrong longitudes when the offset is not a multiple of num_lons
  • none/dyn_grid.F90: the global gw/area array was indexed with a task-local latitude index; now sliced to task-local latitudes like the latitude read
  • gravity_wave_drag_ridge_read.F90: fh_rdggm was a disassociated pointer passed to cam_pio_openfile and later deallocated without ever being allocated; now a plain locally-owned file handle
  • gravity_wave_drag_ridge_read.F90: rdg_gbxarg was left all zeros (while marked initialized) when GBXAR came from the topo file, causing divide-by-zero in the meso-Gamma ridge scheme
  • cam_hist_file.F90: history output buffer could contain uninitialized memory for fields with zero accumulated samples; now initialized to the field fill value
  • dp_coupling.F90: removed OpenMP parallel-do over k on the wet/dry interface-pressure recurrences (loop-carried dependence in k plus a race on the ps accumulation); upstream CAM computes these serially
  • phys_comp.F90, cam_comp.F90: dtime_phys (timestep_for_physics) has no registry initial value, so allocate_physics_types_fields set it to NaN after cam_init seeded it; CCPP init phases that consume it (rrtmgp_inputs_setup with use_rad_dt_cosz) saw NaN. Seed it in phys_init after the allocation instead; stepon_timestep_init still updates it per timestep
  • physics_data.F90: check_field_2d/3d tested only the model state for NaN, so NaNs in the snapshot file were reported as "no differences"
  • physics_grid.F90: memory highwater message printed mem_end
  • orbital_data.F90: acos of shr_orb_cosz could be evaluated outside [-1,1]
  • ref_pres.F90: mark_as_initialized for nbot_molec used a standard name that does not exist in ref_pres.meta
  • none/dyn_grid.F90: unsupported grid-area dimension length built an error message but never aborted; per-column areas on a lat/lon grid reached pio_read_darray with an unassociated iodesc
  • none/dyn_grid.F90: find_dimension used len() instead of size() on the dim_names array
  • ic_baroclinic.F90: optional dummy verbose referenced without a present() guard; evaluate_streamfunction dummy argument order did not match its call sites
  • dyn_thermo.F90: get_exner read exner_phys after deallocating it
  • se/dp_coupling.F90, se/stepon.F90: removed dp3d_tmp_tmp and diag_dynvar_ic qtmp, both allocated/filled and never read
  • atm_comp_nuopc.F90: InitializeRealize restored the share log unit from an uninitialized variable; DataInitialize early return left it pointing at the CAM log
  • atm_comp_nuopc.F90: CAM/driver clock consistency check used .and. so a desync in date or time alone was not detected
  • atm_comp_nuopc.F90: ESMF_MeshGet and two ESMF_ClockGet calls omitted rc=, so the following ChkErr tested a stale return code
  • atm_comp_nuopc.F90: cam_write_srfrest data-write loop used a stale gridToFieldMap from the define loop
  • atm_stream_ndep.F90: ndep units string could be tested while uninitialized when the units attribute is missing
  • cam_hist_file.F90: config_set_beg_time truncated the sub-day part of the interval start time (seconds_per_day was declared integer)
  • cam_hist_file.F90: interpolated-output namelist values (hist_interp_out, hist_interp_nlat, hist_interp_nlon) were not broadcast
  • cam_history.F90: 'second' output frequency used truncating integer division while all other units use nint
  • cam_history.F90: dimbounds is only partially filled by cam_grid_get_array_bounds and the undefined half was stored in the field
  • cam_grid_support.F90: two bare allocates passed an uninitialized status to check_allocate, which can abort a healthy run
  • cam_grid_support.F90: cam_grid_find_src_dims left src_out undefined when a grid coordinate name matched no field dimension; now aborts with the name
  • cam_grid_support.F90: cam_grid_patch_write_vals created two PIO decompositions but freed only the second
  • tracer_data.F90: open_trc_datafile left optional intent(out) cyc_ndx_beg/cyc_ndx_end undefined when the cycle year is absent, defeating its own not-found check
  • cam_time_coord.F90: time_coordinate%copy omitted dtime, time_interp, wghts and indxs, leaving the copy unusable; dtime also had no default
  • gmean_mod.F90: gmean_float_norepro divided and printed check_sum on non-root ranks where MPI_reduce leaves it undefined
  • time_manager.F90: Gregorian leap-day fold missed calday exactly equal to 366.0 in get_curr_calday and get_calday
  • radiative_aerosol.F90: rad_aer_get_info_by_bin indexed bins%names with the list index instead of the bin-definition index

Describe any changes made to build system:

Describe any changes made to the namelist:

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

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:

derecho/gnu/aux_sima:

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

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:

- ic_us_standard_atm.F90: nlev was never set for the T block, causing
  zero-size work arrays and uninitialized temperature
- ic_us_standard_atm.F90: constituent loop compared the global constituent
  index against a findloc position and indexed Q with the global index
- none/dyn_grid.F90: lon_index ignored the task's global column offset,
  assigning wrong longitudes when the offset is not a multiple of num_lons
- none/dyn_grid.F90: the global gw/area array was indexed with a task-local
  latitude index; now sliced to task-local latitudes like the latitude read
- gravity_wave_drag_ridge_read.F90: fh_rdggm was a disassociated pointer
  passed to cam_pio_openfile and later deallocated without ever being
  allocated; now a plain locally-owned file handle
- gravity_wave_drag_ridge_read.F90: rdg_gbxarg was left all zeros (while
  marked initialized) when GBXAR came from the topo file, causing
  divide-by-zero in the meso-Gamma ridge scheme
- cam_hist_file.F90: history output buffer could contain uninitialized
  memory for fields with zero accumulated samples; now initialized to the
  field fill value
- dp_coupling.F90: removed OpenMP parallel-do over k on the wet/dry
  interface-pressure recurrences (loop-carried dependence in k plus a race
  on the ps accumulation); upstream CAM computes these serially
Small self-contained fixes; issue and fix are one-to-one unless noted.

Control / physics infrastructure:
- phys_comp.F90, cam_comp.F90: dtime_phys (timestep_for_physics) has no
  registry initial value, so allocate_physics_types_fields set it to NaN
  after cam_init seeded it; CCPP init phases that consume it (rrtmgp_inputs
  _setup with use_rad_dt_cosz) saw NaN. Seed it in phys_init after the
  allocation instead; stepon_timestep_init still updates it per timestep
- physics_data.F90: check_field_2d/3d tested only the model state for NaN,
  so NaNs in the snapshot file were reported as "no differences"
- physics_grid.F90: memory highwater message printed mem_end
- orbital_data.F90: acos of shr_orb_cosz could be evaluated outside [-1,1]
- ref_pres.F90: mark_as_initialized for nbot_molec used a standard name
  that does not exist in ref_pres.meta

Dynamics:
- none/dyn_grid.F90: unsupported grid-area dimension length built an error
  message but never aborted; per-column areas on a lat/lon grid reached
  pio_read_darray with an unassociated iodesc
- none/dyn_grid.F90: find_dimension used len() instead of size() on the
  dim_names array
- ic_baroclinic.F90: optional dummy verbose referenced without a present()
  guard; evaluate_streamfunction dummy argument order did not match its
  call sites
- dyn_thermo.F90: get_exner read exner_phys after deallocating it
- se/dp_coupling.F90, se/stepon.F90: removed dp3d_tmp_tmp and diag_dynvar_ic
  qtmp, both allocated/filled and never read

Coupler:
- atm_comp_nuopc.F90: InitializeRealize restored the share log unit from an
  uninitialized variable; DataInitialize early return left it pointing at
  the CAM log
- atm_comp_nuopc.F90: CAM/driver clock consistency check used .and. so a
  desync in date or time alone was not detected
- atm_comp_nuopc.F90: ESMF_MeshGet and two ESMF_ClockGet calls omitted rc=,
  so the following ChkErr tested a stale return code
- atm_comp_nuopc.F90: cam_write_srfrest data-write loop used a stale
  gridToFieldMap from the define loop
- atm_stream_ndep.F90: ndep units string could be tested while
  uninitialized when the units attribute is missing

History:
- cam_hist_file.F90: config_set_beg_time truncated the sub-day part of the
  interval start time (seconds_per_day was declared integer)
- cam_hist_file.F90: interpolated-output namelist values (hist_interp_out,
  hist_interp_nlat, hist_interp_nlon) were not broadcast
- cam_history.F90: 'second' output frequency used truncating integer
  division while all other units use nint
- cam_history.F90: dimbounds is only partially filled by
  cam_grid_get_array_bounds and the undefined half was stored in the field

Utilities:
- cam_grid_support.F90: two bare allocates passed an uninitialized status to
  check_allocate, which can abort a healthy run
- cam_grid_support.F90: cam_grid_find_src_dims left src_out undefined when a
  grid coordinate name matched no field dimension; now aborts with the name
- cam_grid_support.F90: cam_grid_patch_write_vals created two PIO
  decompositions but freed only the second
- tracer_data.F90: open_trc_datafile left optional intent(out)
  cyc_ndx_beg/cyc_ndx_end undefined when the cycle year is absent,
  defeating its own not-found check
- cam_time_coord.F90: time_coordinate%copy omitted dtime, time_interp,
  wghts and indxs, leaving the copy unusable; dtime also had no default
- gmean_mod.F90: gmean_float_norepro divided and printed check_sum on
  non-root ranks where MPI_reduce leaves it undefined
- time_manager.F90: Gregorian leap-day fold missed calday exactly equal to
  366.0 in get_curr_calday and get_calday

Aerosol:
- radiative_aerosol.F90: rad_aer_get_info_by_bin indexed bins%names with the
  list index instead of the bin-definition index
@jimmielin jimmielin self-assigned this Sep 11, 2026
@jimmielin jimmielin added the bug-fix This PR was created to fix a specific bug. label Sep 11, 2026
Comment thread src/data/ref_pres.F90
call mark_as_initialized("do_molecular_diffusion")
! nbot_molec
call mark_as_initialized("index_of_pressure_at_bottom_of_molecular_diffusion")
call mark_as_initialized("vertical_layer_index_at_bottom_of_molecular_diffusion")

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.

this is the stdname in the .meta for gw in atmos_phys which I assume is the canonical one

@jimmielin
jimmielin deployed to CI-tests-on-CIRRUS September 11, 2026 15:05 — with GitHub Actions Active

This branch is waiting to be deployed

1 waiting deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix This PR was created to fix a specific bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants