Phase-3: Riemann hot-path decomposition into shared GPU device helpers

[sbryngelson]

Stacked on #1551 → #1552 → #1553 → #1555 → #1556 — its own content is the top 4 commits. Completes the refactoring roadmap's Phase 3 (hot-path decomposition), the final phase of the series.

Summary

1. Per-solver benchmark coverage. 5eq_rk3_weno3_hll and 5eq_rk3_weno3_lf join the benchmark suite (previously only HLLC and hypo-HLL had solver-specific coverage), so the performance gates below actually measure every solver the changes touch.

2. Tier-1 shared device helpers (m_riemann_state.fpp, all GPU_ROUTINE(seq, cray_inline)): mixture-properties aggregation (14 call sites across HLL/HLLC/LF), interface-Reynolds averaging (10 sites), and the HLLC star-momentum flux expression (6 sites). Net −50 LOC from the hot loops. Each conversion verified bitwise-identical on live lanes by independent review; blocks whose arithmetic genuinely differs between solvers were deliberately left inline rather than homogenized. Incidental fix: dead viscous lanes previously stored 2/(1/uninitialized) into Re_avg — now a defined value (fixes the UB/FP-trap hazard).

3. Hypoelastic interface energy unified under maintainer rulings (recorded in-repo): one shared helper with the verysmall gate (HLL's G > 1000 stability floor and its TODO take out comment retired), unconditional damage scaling, and the dimension-correct shear_indices doubling test; the dead hypoelastic energy blocks in HLLC (×2) and LF deleted outright (the case validator restricts hypoelasticity to HLL). Net −56 LOC. Wave-speed estimates untouched everywhere.

Performance gate (<5% ceiling, maintainer-set)

Simulation grind-time ratios vs. the pre-series baseline, all benchmark cases:

stage	range	watched case
Tier-1 helpers	0.99–1.04	5eq_rk3_weno3_hllc 1.02 (faster)
Hypoelastic unification	0.98–1.03	hypo_hll 1.03 (faster)

(Ratios are speedups: >1 is faster. Pre/post exec scatter is sub-second I/O noise, documented in the working notes.)

Behavior notes

• Golden suite green (4-way sharded) after each stage — the changes are golden-test-silent, but two are real for untested regimes and deliberate: soft-G hypoelastic runs (verysmall < G ≤ 1000) now receive elastic energy the stability floor previously suppressed (maintainer ruling), and dead-lane Re_avg values changed from garbage to defined (consumers gated; results-inert).
• Fixes HLLC hypoelastic energy block mis-doubles the theta-theta normal stress in 2D-axisymmetric (currently dead code) #1566 (the deleted HLLC blocks contained the hard-coded axisym shear-doubling bug).
• Fixes Uninitialized read into Re_avg on dead viscous lanes (UB; FP-trap hazard in debug builds) #1567 (the uninitialized Re_avg lanes).
• Deliberately NOT touched, tracked for physics rulings: Hyperelastic wave-speed estimates: HLLC includes the tau_e correction, HLL deliberately omits it #1557, HLL hypoelastic damage load reads the LEFT cell's damage for the RIGHT state (missing +1 offset) #1570, HLL hyperelastic wave-speed branch reads shear moduli that only the hypoelastic path initializes #1571 (elastic wave-speed formulation divergence, the damage-R load offset, HLL's hyperelastic G initialization).

Docs

gpuParallelization.md gains a "Writing GPU_ROUTINE Device Helpers" section (the macro was previously undocumented): the FLOP threshold, the seq+cray_inline idiom and why Cray requires it, the caller-loads/helper-computes pattern, AMD case-optimization dimensioning, and declare-scoping — with a real helper from this PR as the worked example.

[Copilot]

Pull request overview

This PR is the final stage of a stacked refactor series that decomposes the Riemann-solver hot paths into shared GPU device helpers, tightens multi-rank MPI broadcast correctness via registry-driven generation, and updates build/docs infrastructure to support these changes and performance gating.

Changes:

• Extracts shared, inlined GPU_ROUTINE(seq, cray_inline) device helpers (mixture aggregation, interface Reynolds averaging, hypoelastic energy, HLLC star momentum flux) and reduces solver hot-loop duplication.
• Moves MPI broadcast lists toward registry-driven generation while preserving manual “irregular” residues (struct roots, nested member patterns), addressing prior multi-rank divergence/UB hazards.
• Refactors CMake build plumbing into modular .cmake includes and documents the GPU helper pattern.

Reviewed changes

Copilot reviewed 42 out of 45 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/simulation/m_riemann_state.fpp	Adds shared GPU device helpers and consolidates per-sweep Riemann state logic.
toolchain/mfc/params/generators/fortran_gen.py	Implements generated MPI broadcast include emission and supporting classification/type-mapping helpers.
cmake/MFCTargets.cmake	Centralizes target setup/linking logic for MPI/OpenACC/OpenMP/GPU builds.
cmake/Fypp.cmake	Wires Fypp preprocessing with explicit generated-include dependencies.
cmake/ParamsCodegen.cmake	Adds build-time (ninja-tracked) generation of Fortran parameter include fragments.
cmake/GPU.cmake	Collects compiler/GPU-related flags and IPO/LTO handling into a dedicated module.

[sbryngelson]

Thanks Copilot — all five findings verified and addressed in b43e445. Context: cmake/MFCTargets.cmake was created by pure motion from the monolithic CMakeLists.txt (the split was verified by byte-identical configure graphs), so all five lines pre-exist on master — but four were worth fixing on substance: the unified-memory flags (the NVHPC managedalloc compile/link pair and both FRONTIER_UNIFIED definitions) now apply to ${a_target} so the NVHPC two-pass IPO lib target receives them consistently, and the leftover debug message(STATUS) is gone. The set(ENV{OMP_TARGET_OFFLOAD} [MANDATORY]) line was deleted outright rather than de-bracketed: set(ENV{...}) only affects the configure-time CMake process environment, so it never influenced the built binaries regardless of the bracket bug.

[codecov]

Codecov Report

❌ Patch coverage is 79.76190% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.81%. Comparing base (b4be438) to head (f3c96cc).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/simulation/m_riemann_solver_hllc.fpp	58.62%	12 Missing ⚠️
src/simulation/m_riemann_state.fpp	92.10%	0 Missing and 3 partials ⚠️
src/simulation/m_riemann_solver_hll.fpp	81.81%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1572      +/-   ##
==========================================
+ Coverage   60.51%   60.81%   +0.30%     
==========================================
  Files          83       83              
  Lines       19905    19793     -112     
  Branches     2950     2928      -22     
==========================================
- Hits        12046    12038       -8     
+ Misses       5866     5772      -94     
+ Partials     1993     1983      -10     

☔ 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.

[sbryngelson]

Code review

Found 2 issues, both fixed in fc7f770:

1. The hypoelastic energy-block deletions left LF and HLLC with hypoelasticity-gated flux/source blocks consuming tau_e_L/R that nothing loads (flagged independently by three review passes). Unreachable through the toolchain (case_validator.py:577 restricts hypoelasticity to HLL), but a latent-UB trap if that restriction is ever lifted. Resolved by completing the dead-copy deletion under the same maintainer ruling: every hypoelasticity-gated block in LF/HLLC removed after a guard-by-guard inventory; all elasticity/hyperelasticity-gated code untouched (with HLLC, elasticity implies hyperelasticity, so the kept consumers are always fed by the kept loads). −60 LOC of dead code, plus orphaned locals.

https://github.com/MFlowCode/MFC/blob/a577f15e09134f4f12bd49646bee9ed8fcab84a0/src/simulation/m_riemann_solver_lf.fpp#L443-L450

2. The new GPU_ROUTINE documentation overstated cray_inline=True as required "always" while two same-module helpers in the same file correctly omit it, and misplaced the AMD dimensioning guard as "caller-side". Both passages corrected (cray_inline scoped to cross-module helpers; the guard sits on the helper's dummy declaration), and the hypoelastic helper's doc comment now records the deliberate verysmall-gate ruling.

https://github.com/MFlowCode/MFC/blob/a577f15e09134f4f12bd49646bee9ed8fcab84a0/docs/documentation/gpuParallelization.md#L636-L640

Verified sound across five independent review passes: helper algebra bitwise-faithful at all 30 call sites (sampled), no upstream hotfixes lost in any replaced block (history audit vs #1545/#1556 merges), Herschel-Bulkley viscous arguments preserved, cmake a_target fixes correct, all five prior Copilot points confirmed addressed, merge-base current. Known and deliberately preserved pending physics rulings: #1570 (damage-R offset), #1557/#1571 — declared in the PR description.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[sbryngelson]

Device validation complete on the current head's content (860ecb3b1): full 594-test suite run on GPU nodes under both backends — OpenACC and OpenMP offload — ALL TESTS PASSED on each, alongside the 4-way CPU golden gate. Every change in this PR now has empirical GPU evidence in addition to the emission-level proofs.