Reject truncated restart files and incompatible boundary decompositions

[sbryngelson]

Summary

Truncated restart files and per-rank boundary files written for another MPI decomposition could be read silently as invalid simulation inputs. This PR combines file-size validation with boundary-decomposition validation and checked boundary-file opens. Post-processing retains the documented warning behavior for decomposition mismatches, and legacy boundary files without decomposition metadata remain compatible.

Consolidation

This existing PR retains its original commits and discussion. Changes from #1947 are added as separate commits with original authors/messages and cherry-pick -x provenance. The complete source PR descriptions, verification records, and limitations are reproduced below. Their original verification claims are historical records, not fresh runs on this combined head.

Verification of the combined branch

• The previous head of this PR remains an ancestor of the combined head.
• Every added/deleted line in the imported non-merge commits is preserved exactly.
• git diff --check passes excluding generated golden metadata; its original formatting is retained.
• ./mfc.sh precheck passes all seven gates: formatting, spelling, toolchain lint/tests, source lint, documentation references, parameter documentation, and example case validation.
• Full solver regressions and MPI reproductions were not repeated for this consolidation. Original records remain below, and CI must validate the combined head.

This consolidation was performed with OpenAI Codex.

Contribution Policy

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

All contributions are expected to demonstrate:

• A clear understanding of the codebase
• Alignment with product direction
• Thoughtful reasoning behind changes
• Evidence of real-world usage or hands-on experience with the problem

If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.

---

Acknowledgement

• I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

---

PR template credit: junegunn

Original PR documentation

#1948: Restart: abort on a truncated restart file instead of reading garbage

Source: #1948

Original head: d0c59cd90ed0fd26ae4230ff5aa3b86f272897a3

Complete original PR description

Description

A restart file truncated by a job killed while writing it (restart_data/lustre_<step>.dat) is read without any check, and the run either starts from garbage (in production: Inf/NaN pressure) or dies without saying why.

Root cause. s_read_parallel_data_files (src/simulation/m_start_up.fpp) opens the restart file and reads sys_size variables (plus the qbmm pb/mv when present) at offsets computed from the global grid. It never compares the file size to what it reads. An MPI read past end-of-file returns short without raising an error, so the missing tail comes back as whatever was in the buffer.

Fix. The new helper s_check_restart_file_size compares MPI_FILE_GET_SIZE against the bytes about to be read, and aborts with a message naming the file and both sizes. It is called in both the shared-file and the file_per_process branches. It only checks for a file that is too short, so files with extra trailing data (e.g. Lagrangian beta) still read. Intact files are unaffected.

Verification

Production (OLCF Frontier). A job killed during a save left a short lustre_<step>.dat. The next restart read it as Inf pressure. The workaround was to pick the latest restart whose size equals nx·ny·nz·sys_size·8 bytes.

This branch. I ran this on a Frontier CPU compute node (GNU 12.3 + Cray MPICH, Release) with a 2D 50x40 case on 2 ranks. I ran to step 20 with saves every 10 steps, truncated lustre_10.dat from 80000 to 40000 bytes, and restarted from step 10.

code	restart from the truncated file
this branch	aborts: Restart file ./restart_data/lustre_10.dat holds 40000 of 80000 expected bytes. It is truncated, e.g. by a job killed while writing it; restart from an earlier step.
master	dies with exit code 255, with no message naming the file

Restarting from the intact file on this branch runs normally.

simulation compiles with CCE 19 (CPU) and GNU 12.3. Precheck passes, apart from two test_thermochem cases that fail on the Frontier login node because they compile with the system /usr/bin/gfortran (addressed by #1943). Existing goldens are unaffected, because they read intact files. No regression test is added: the harness cannot express an expected abort.

Contribution Policy

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

All contributions are expected to demonstrate:

• A clear understanding of the codebase
• Alignment with product direction
• Thoughtful reasoning behind changes
• Evidence of real-world usage or hands-on experience with the problem

If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.

---

Acknowledgement

• I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

---

The problem was hit in production runs on Frontier; the fix was exercised on the small case above.

PR template credit: junegunn

#1947: BC I/O: refuse per-rank boundary files written for another decomposition

Source: #1947

Original head: 3ed989ac96da521d9d210c1c3de084034b5d0d10

Complete original PR description

Description

Restarting (or just running simulation) on a different rank count from pre_process silently corrupts Dirichlet (-17) inflow and boundary-patch data.

Root cause. When bc_io is on (any -17 boundary or num_bc_patches > 0), pre_process writes the boundary types and buffers per rank, to restart_data/boundary_conditions/bc_<rank>.dat (s_write_parallel_boundary_condition_files). simulation and post_process open bc_<proc_rank>.dat by their own rank index and read it unchecked (s_read_parallel_boundary_condition_files). The restart file itself is in a global layout and reads on any rank count, so the only thing tying a run to the pre_process decomposition is this directory, and nothing checks it. On another rank count each rank gets another rank's boundary slab, with the wrong position and possibly the wrong size. The per-rank MPI_File_open was not checked either, so a missing file went unnoticed too.

Fix (minimal).

• pre_process rank 0 also writes restart_data/boundary_conditions/decomposition.dat, containing num_procs num_procs_x num_procs_y num_procs_z.
• simulation aborts on a mismatch, with a message that names both decompositions.
• post_process only uses the data for boundary ghost cells, so it warns instead of aborting (new optional strict argument). Workflows that post-process on a different rank count keep working, but are told that boundary ghost values are wrong.
• Files written before this change have no decomposition.dat and are read as before, unchecked.
• Both per-rank MPI_File_open calls now go through s_check_mpi_file_open.

Not done here: the full fix. The full fix would write and read these arrays in a global parallel-I/O layout, like the restart file, so that any rank count works. I did not do it in this PR because it is not contained:

• the direction-2 and direction-3 arrays carry -buff_size:m+buff_size halos, which overlap between ranks and include corner cells, and buff_size can differ between targets;
• each face would need its own global subarray type, and only the ranks on that physical boundary hold data for it;
• it changes the file format for all three targets, so old files would need a fallback.

Verification

Production (OLCF Frontier, CCE 19, OpenACC). pre_process ran on 192 ranks and the restart ran on 64 ranks at step 15714. The inflow cell j = 0 reached ICFL 1.015 at step 15723. In a second run (360 ranks, then 240), p = -4e47 appeared at the inflow face at step 2. Runs that kept the rank count were unaffected.

This branch. I ran these on a Frontier CPU compute node (GNU 12.3 + Cray MPICH, Release). The case is a 2D uniform channel, 50x40, with bc_x%beg = -17.

pre_process	simulation	result
2 ranks, this branch	2 ranks, this branch	runs; decomposition.dat = 2 2 1 1
2 ranks, this branch	1 rank, this branch	aborts: ./restart_data/boundary_conditions was written for 2 ranks (2x1x1) but this run uses 1 ranks (1x1x1). These per-rank boundary files only work on the decomposition that wrote them: run on that rank count, or rerun pre_process on this one.
2 ranks, master	1 rank, master	runs to the end with no warning, reading the 2-rank files
2 ranks, master (no decomposition.dat)	2 ranks, this branch	runs (backward compatible)

Builds: pre_process, simulation and post_process compile with CCE 19 on CPU, and pre_process and simulation with GNU 12.3. The post_process warning path was compiled but not run. Precheck passes, apart from two test_thermochem cases that fail on the Frontier login node because they compile with the system /usr/bin/gfortran (addressed by #1943).

No regression test is added. The test harness compares goldens and has no way to expect an abort, and a test that restarts on a different rank count would need harness support.

Contribution Policy

We do not accept pull requests generated primarily by AI without genuine understanding or real-world usage context.

All contributions are expected to demonstrate:

• A clear understanding of the codebase
• Alignment with product direction
• Thoughtful reasoning behind changes
• Evidence of real-world usage or hands-on experience with the problem

If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.

---

Acknowledgement

• I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

---

The bug was found in production runs on Frontier; the fix was exercised on the small cases above.

PR template credit: junegunn

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open findings

• n_MOK is computed from m_glb_read instead of n_glb_read. This can make the computed… · New
• The expression 2*nb*nnode is evaluated in default integer arithmetic before being added to… · New
• ierr from MPI_FILE_GET_SIZE is not checked. If this MPI call fails, file_bytes may be… · New
• The phrase 'holds <file_bytes> of <expected_bytes> expected bytes' is a bit grammatically… · New

What changed in this PR

Adds a restart-file size sanity check to prevent silent short MPI reads from truncated restart files, aborting with a clear error message instead of proceeding with garbage data.

Changes:

• Compute the number of variables per cell to be read (including optional QBMM data) as an MPI-offset-sized integer.
• Check restart file size via MPI_FILE_GET_SIZE in both file_per_process and shared-file branches before reading.
• Introduce s_check_restart_file_size helper to centralize truncation detection and abort messaging.

File	Description
src/​simulation/​m_start_up.fpp	Adds pre-read restart file size validation and a helper routine to abort on truncated restart files.

🧠 Review effort: Lite

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.79%. Comparing base (3dc5b2f) to head (4612c60).
⚠️ Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
src/simulation/m_start_up.fpp	60.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1948      +/-   ##
==========================================
- Coverage   62.64%   61.79%   -0.85%     
==========================================
  Files          86       86              
  Lines       22425    22783     +358     
  Branches     3325     3355      +30     
==========================================
+ Hits        14048    14079      +31     
- Misses       6119     6214      +95     
- Partials     2258     2490     +232     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Lines of Code

File	Lines	Diff
src/simulation/m_start_up.fpp	1267	+23

Directory	Lines	Diff
simulation	28077	+23
total	47031	+23