Skip to content

Check and enforce FLOAT64 snapshots for null dycore#478

Open
jimmielin wants to merge 4 commits intoESCOMP:developmentfrom
jimmielin:hplin/no_float32_snapshots
Open

Check and enforce FLOAT64 snapshots for null dycore#478
jimmielin wants to merge 4 commits intoESCOMP:developmentfrom
jimmielin:hplin/no_float32_snapshots

Conversation

@jimmielin
Copy link
Copy Markdown
Member

@jimmielin jimmielin commented Mar 13, 2026

Tag name (required for release branches):
Originator(s): @jimmielin co-authored with claude-opus-4.6

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

  • Enforce 64-bit snapshots being read into the null dycore
    If the snapshot is not written out with ndens=1 on the CAM end it will cause incomprehensible answer differences comparing CAM and CAM-SIMA. So if a FLOAT32 snapshot is detected now the model will crash:
dec2442.hsn.de.hpc.ucar.edu 0:  model_grid_init: Snapshot file has non-FLOAT64 data (variable state_u). This will cause answer differences!Please rerun CAM with ndens = 1 to write 64-bit float snapshots for use with CAM-SIMA.

This only applies when ncdata_check is set, in case the same null dycore functionality is used for non-CAM snapshot runs (thanks to @nusbaume for the suggestion)

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): N/A

List all files eliminated and why: N/A

List all files added and what they do: N/A

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/dynamics/none/dyn_grid.F90
  - add a PIO_DOUBLE check on the existing check for U/state_u/eastward_wind

M.      src/physics/phys_comp.F90
  - make ncdata_check public for access by dyn_grid.F90.

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:

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:

@jimmielin jimmielin self-assigned this Mar 13, 2026
@jimmielin jimmielin added the enhancement New feature or request label Mar 13, 2026
@jimmielin jimmielin temporarily deployed to CI-tests-on-CIRRUS March 13, 2026 16:29 — with GitHub Actions Inactive
@jimmielin jimmielin temporarily deployed to CI-tests-on-CIRRUS March 13, 2026 16:52 — with GitHub Actions Inactive
Copy link
Copy Markdown
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Thanks for the additional check @jimmielin! I just had one change request, which is basically that this check should only be done if we know we are running a snapshot test by examining the ncdata_check namelist variable as well.

@jimmielin jimmielin temporarily deployed to CI-tests-on-CIRRUS March 15, 2026 22:03 — with GitHub Actions Inactive
@jimmielin jimmielin requested a review from nusbaume March 15, 2026 22:23
Copy link
Copy Markdown
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Thanks @jimmielin! Everything looks good to me now, although I did have one (optional) variable declaration order request.

@jimmielin jimmielin temporarily deployed to CI-tests-on-CIRRUS March 16, 2026 17:27 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants