Generalize checkpoints to support arbitrary payloads - #2676
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors Neko’s checkpointing to be payload-based (arbitrary named collections of fields/series/arrays/mesh-arrays), enabling extensible HDF5 checkpoint layouts while preserving legacy .chkp compatibility via a positional “legacy view”. It also adds restart support across polynomial orders for HDF5 checkpoints (including ALE) and introduces new unit/integration tests plus documentation updates.
Changes:
- Replaces the monolithic
chkp_tpointer layout with an array ofcheckpoint_payload_tregistrations, including time-history registration via a dedicated payload. - Updates HDF5 and legacy
.chkpcheckpoint I/O to write/read payload hierarchies and map payloads back into the legacy.chkppositional format. - Adds tests (pFUnit + pytest integration) and updates user documentation/CHANGELOG to reflect new checkpoint formats and capabilities.
Reviewed changes
Copilot reviewed 30 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/Makefile.am | Adds checkpoint_payload unit-test subdir and test target. |
| tests/unit/checkpoint_payload/test_checkpoint_payload.pf | New pFUnit coverage for payload registration and lookup semantics. |
| tests/unit/checkpoint_payload/Makefile.in | Build rules for the new checkpoint payload unit test. |
| tests/unit/.gitignore | Ignores the new unit-test executable artifact. |
| tests/integration/tests/test_scalar_restart/test_scalar_restart.py | New integration test for scalar restart continuity (+ HDF5 p-interp variant). |
| tests/integration/tests/test_scalar_restart/scalar_restart.case | New case file used by scalar restart integration tests. |
| tests/integration/tests/test_scalar_restart/reference_part1_cpu_sp.csv | Reference convergence data for scalar restart (part 1, CPU SP). |
| tests/integration/tests/test_scalar_restart/reference_part1_cpu_dp.csv | Reference convergence data for scalar restart (part 1, CPU DP). |
| tests/integration/tests/test_scalar_restart/reference_part2_cpu_sp.csv | Reference convergence data for scalar restart (part 2, CPU SP). |
| tests/integration/tests/test_scalar_restart/reference_part2_cpu_dp.csv | Reference convergence data for scalar restart (part 2, CPU DP). |
| tests/integration/tests/test_ale_restart/test_ale_restart.py | New integration test for ALE HDF5 restart across polynomial orders. |
| tests/integration/.gitignore | Keeps scalar restart reference CSVs tracked despite global CSV ignore. |
| src/simulation_components/user_stats.f90 | Minor import alignment/formatting adjustment. |
| src/scalar/scalars.f90 | Moves scalar IC handling into scalars_t and removes legacy scalar checkpoint registration logic. |
| src/scalar/scalar_scheme.f90 | Adds payload registration + scalar IC setter; changes params handling to an owned copy. |
| src/scalar/scalar_scheme_fctry.f90 | Ensures scalar schemes register checkpoint payloads after init. |
| src/scalar/scalar_pnpn.f90 | Registers Pn/Pn-specific checkpoint fields and switches time-history access to the new API. |
| src/Makefile.am | Adds common/checkpoint_payload.f90 to build sources. |
| src/io/hdf5_file.F90 | Implements payload read/write for checkpoints and adds p-interpolation read paths. |
| src/io/chkp_file.f90 | Adds legacy_checkpoint_view_t and maps payloads into legacy .chkp layout. |
| src/fluid/fluid_scheme_incompressible.f90 | Adds validation checks for consistent velocity spaces / mesh compatibility. |
| src/fluid/fluid_pnpn.f90 | Registers fluid checkpoint payload and switches time-history access to the new API. |
| src/common/checkpoint.f90 | Overhauls chkp_t to manage payload registrations and time-history payload. |
| src/common/checkpoint_payload.f90 | New payload container type supporting fields/series/arrays/mesh-arrays. |
| src/case.f90 | Registers time-history via checkpoint API and delegates scalar IC setup to scalars_t. |
| src/ale/ale_manager.f90 | Registers ALE state via payloads and removes legacy-format restriction. |
| src/.depends | Updates build dependencies for the new payload module and modified compilation units. |
| doc/pages/user-guide/io.md | Documents .chkp vs HDF5 checkpoints and new capabilities/limits. |
| doc/pages/user-guide/case-file.md | Updates restart_file suffix rules and ALE restart documentation. |
| configure.ac | Adds the new unit-test Makefile to autotools generation. |
| CHANGELOG.md | Adds an entry describing the new payload-based checkpointing and HDF5 enhancements. |
| .gitignore | Ignores example-generated .h5 files. |
Comments suppressed due to low confidence (2)
src/io/hdf5_file.F90:752
- The interpolation path reads the checkpoint data into a 1D buffer (
checkpoint_data(:)) and then callsinterpolator_t%map_host, whose interface requires rank-4 arrays shaped by the source/target spaces. As written, this is a rank mismatch that should not compile, andspace_interp%init(fld%Xh, source_Xh)appears to initialise the interpolator with source/target spaces reversed. Use a localblockto remap the flat buffer to a 4D pointer and initialise the interpolator as(source_Xh, fld%Xh)before mapping intofld%x.
! Interpolation across spaces required!
dcount(1) = int(fld%msh%nelv, hsize_t) * &
int(source_Xh%lxyz, hsize_t)
doffset(1) = int(fld%msh%offset_el, hsize_t) * &
int(source_Xh%lxyz, hsize_t)
src/io/hdf5_file.F90:916
- In the mesh-array interpolation branch,
stored_data(:)/array%x(:)are rank-1 arrays, butinterpolator_t%map_hostrequires rank-4 arrays. This should not compile (rank mismatch), andspace_interp%init(array%Xh, stored_Xh)appears to reverse the source/target spaces. Remap both the stored buffer andarray%xto rank-4 pointers in a localblock, then initialise the interpolator as(stored_Xh, array%Xh)and map into the remapped destination view.
else
allocate(stored_data(int(dcount(1))))
call h5dread_f(dset_id, h5_neko_real, stored_data, dcount, ierr, &
file_space_id = filespace, mem_space_id = memspace, &
xfer_prp = plist_id)
|
Very nice @timofeymukha. |
Both branches reworked scalar checkpointing independently. This branch replaced the fixed pointer members of chkp_t with named payloads, while develop threaded a checkpoint index and scalar count through the scalar factory to index the old fixed storage. The payload registry supersedes that, so chkp_add_scalar, chkp_add_ale and the index plumbing through scalars.f90, scalar_scheme.f90 and scalar_scheme_fctry.f90 are gone. Changes carried over from develop: - The dofmap coordinates are tensor4_t, so ale_manager reaches the nodal arrays and their device pointers through %x and %x_d. - The time history is double precision everywhere, including in the legacy .chkp records, which now use MPI_DOUBLE_PRECISION. Payload arrays gained double-precision storage in checkpoint_array_t%x_dp to carry it. The registration routine for it is add_array_dp rather than another specific of the add_array generic, because rp and dp are the same kind in a double-precision build and the generic would then be ambiguous. The HDF5 array reader and writer select the datatype and the broadcast type from which pointer is associated. - HDF5 library session management, and separate identifiers for the file-access property list and for each dataset's own dataspace. The mesh-to-mesh restart develop added to the HDF5 reader is not yet wired into the payload reader, which interpolates across polynomial orders only. Its helper, hdf5_read_field, is kept unchanged in hdf5_file.F90 as the basis for that work; the setup block that drove it has to be rebuilt against the payload reader and was not carried over. The time history was read but never used in fluid_pnpn_restart and scalar_pnpn_restart, so those blocks are removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The time history is registered through add_array_dp, which keeps double precision whatever the working precision is, and the checkpoint readers and writers tell the two kinds of array apart by which of the descriptor's host pointers is associated. Cover that invariant, the metadata of both a replicated and a distributed registration, the aliasing that lets a restart deliver the values it reads back to the caller's storage, and that the two kinds coexist in one payload and survive its release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other unit test makes its objects depend on the Neko library and regenerates its own Makefile through config.status when Makefile.in changes; this one did neither, so after a change under src/ a plain make left the previous test binary in place and the suite silently ran against the library it was linked with rather than the one just built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add two pFUnit tests that run a Taylor-Green vortex for 100 steps, write a checkpoint at step 50, restart from it, and compare every subsequent step against the uninterrupted run. restart_consistency covers the native chkp format; restart_consistency_hdf5 covers HDF5 and is gated behind ENABLE_HDF5. A restart on the same machine, with the same rank count and the same polynomial order, replays the same arithmetic, so the comparison is for exactness rather than against a tolerance. Any difference at all means state was lost, silently regenerated, or passed through an operation it should not have been. Both tests currently fail, and the failure is a real one. The first step after the restart already differs, by an RMSE of about 1e-9 in a single-precision build, and the difference grows monotonically from there. The two formats differ by only a few percent in that error, so the cause lies in the restart path they share rather than in either reader. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both tests compare a restarted run against the uninterrupted one and require the two to agree exactly. That presumes the solver itself is reproducible from one run to the next, and with phmg it is not: two otherwise identical runs, with no checkpoint involved, differ by a few ulp on the device. The TreeAMG coarse solver inside phmg restricts to its coarse levels with atomicAdd (device_masked_atomic_reduction_0 in tree_amg.f90), so the order in which contributions to a coarse node are summed changes between runs. With a Jacobi pressure preconditioner two uninterrupted runs agree exactly, and both restart tests then reproduce the reference run to zero on every step, for the native chkp format and for HDF5. The restart path was never at fault; the earlier commit message describing the failure as a restart defect was mistaken. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The payload specifics took arrays through `contiguous, target` dummies and stored `c_loc` of the dummy for the lifetime of the run. gfortran 12 passes a pointer-array actual to a `contiguous` dummy as a temporary copy, so for the six pointer arrays ale_manager registers (coef%Blag, coef%Blaglag and the four rigid-body arrays) the payload held addresses of memory that was freed on return. A checkpoint written through them carried garbage, and a restart from it wrote the file's data into recycled heap; chkp_file_read then aborted in free(). gfortran 13 and later pass the actual's address, which is why only the gfortran-12 CI jobs failed, on test_ale. Take `c_loc` of the first element of a plain `target` dummy, which no version copies, and check is_contiguous. Declare the `coef` dummies of ale_manager_init and register_checkpoint_fields `target`, so pointers into its components stay defined after return as the standard requires. Reproduced and verified with gfortran 12.4.0 at the CI flags in an Ubuntu 24.04 container: the ALE restart now runs to completion and valgrind reports no invalid accesses. The CUDA build, the restart consistency tests and the ALE restart on the host are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Since the output schedule rework (ExtremeFLOW#2764) a checkpoint is no longer written at the start of a simulation, so the scalar case writes two files, at t=0.05 and t=0.10, and the ALE case one per step. The HDF5 restart tests still expected the initial checkpoint as well and restarted from the second file, which is now the last one. The chkp variant of the scalar test had already been updated. Both HDF5 tests now expect two files and restart from the first, which keeps the restart at t=0.05 for the scalar and at step one for the ALE case. These tests ran in CI for the first time once HDF5 was added to the CPU jobs (ExtremeFLOW#2806); the failures were this expectation, not the restarts, which run to completion from the new files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Dep #2674
This overhauls the chkpt_t type and related functionality.
Old checkpoints should remain valid
Main change and motivation
Previously the type stores a large number of pointers corresponding to various restart fields, like u, v, w, etc. The main issue is that freezes what a checkpoint can contain, unless one goes in and modifies the type. This is a big issue for neko's extensibility, with roots in the legacy custom-format .chkpt format. At the same time the HDF5 checkpoint file, fundamentally, supports a free layout!
In this PR, the chkpt_t now consists of an array of
checkpoint_payloads, which are collections of fields, field series, mesh arrays (a 4D array living on an SEM geometry, used by ALE currently), and ordinary arrays. Generic arrays may be replicated or explicitly distributed. All of these are non-owning of course, and just point to data, like in the original type. Schemes and stuff like ALE simply add payloads to the checkpoint.The hdf5_file, which writes checkpoints, maps each payload to hdf5 group, and the stores the rest as datasets under it.
To load or write a legacy checkpoint file, a compatibility type
legacy_checkpoint_view_tis added that stores all the pointers that were previously in the main type checkpoint type. This structure is populated from the new payload layout in a helper routine calledchkp_build_legacy_view.Additional
The code was written mostly with AI, with iterations on the design from my side. I checked everything manually in the end, with the exception of the low-level hdf5 library calls. I am not versed in that, yet it seems those were very sensitive based on the vtkhdf5 stuff. So if @timfelle, @adperezm or @njansson can take an extra look at that during review it would be great.