Skip to content

Generalize checkpoints to support arbitrary payloads - #2676

Merged
timfelle merged 32 commits into
ExtremeFLOW:developfrom
timofeymukha:feature/scalar_p3
Sep 24, 2026
Merged

timfelle merged 32 commits into
ExtremeFLOW:developfrom
timofeymukha:feature/scalar_p3

Conversation

@timofeymukha

Copy link
Copy Markdown
Collaborator

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_t is 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 called chkp_build_legacy_view.

Additional

  • Support for interpolation across p is added to the hdf5 checkpoints.
  • ALE now supports p-interpolation @arash-mp At least in principle, I am not sure how well an interpolated mass matrix will work and stuff like that.
  • New tests.

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_t pointer layout with an array of checkpoint_payload_t registrations, including time-history registration via a dedicated payload.
  • Updates HDF5 and legacy .chkp checkpoint I/O to write/read payload hierarchies and map payloads back into the legacy .chkp positional 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 calls interpolator_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, and space_interp%init(fld%Xh, source_Xh) appears to initialise the interpolator with source/target spaces reversed. Use a local block to remap the flat buffer to a 4D pointer and initialise the interpolator as (source_Xh, fld%Xh) before mapping into fld%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, but interpolator_t%map_host requires rank-4 arrays. This should not compile (rank mismatch), and space_interp%init(array%Xh, stored_Xh) appears to reverse the source/target spaces. Remap both the stored buffer and array%x to rank-4 pointers in a local block, 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)

Comment thread src/io/hdf5_file.F90 Outdated
Timofey Mukha added 2 commits July 27, 2026 18:24
@arash-mp

Copy link
Copy Markdown
Contributor

Very nice @timofeymukha.
regarding ALE restart with a different poly order, it was already possible with .chkp, but with the (accuracy) cost of setting Blag and Blaglag = B at the restart. Considering that changing poly order will anyway introduce some error and is not considered exact restart. I haven't tested interpolating mass matrix either (not sure even if that's a good idea)
I haven't checked the PR yet, but if it now interpolates the Blag and Blaglag if poly order is changed, I think it's better to leave that as an option, if possible.
also a general question, does it affect both chkp and h5 formats, or works only with h5?

@njansson njansson left a comment

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.

Great stuff 🍻 🐈‍⬛ 🐈

Timofey Mukha and others added 11 commits September 18, 2026 17:09
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>
@timfelle
timfelle enabled auto-merge September 22, 2026 17:28
Timofey Mukha and others added 2 commits September 24, 2026 15:18
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>
@timfelle
timfelle added this pull request to the merge queue Sep 24, 2026
Merged via the queue into ExtremeFLOW:develop with commit 90b5631 Sep 24, 2026
29 checks passed
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.

5 participants