Load-balance, active-box windowing, block-structured AMR - #1628
Open
sbryngelson wants to merge 687 commits into
Open
Load-balance, active-box windowing, block-structured AMR#1628sbryngelson wants to merge 687 commits into
sbryngelson wants to merge 687 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in (“default-off”) family of performance/diagnostic features (load-weight and SFC partition diagnostics, weighted init-time decomposition, rank timing), plus major simulation capabilities (active-box RHS windowing and block-structured AMR) and corresponding post-processing support and documentation/validation updates.
Changes:
- Adds new runtime parameters and toolchain metadata/validation hooks for the experimental performance/AMR feature family.
- Extends the simulation code with new modules for active-box restriction, load-weight diagnostics, SFC partition reporting, rank timing, and AMR integration points (including restart/output plumbing).
- Updates post_process to read/write AMR fine-block overlays and adds/updates golden metadata plus documentation/indexing.
Reviewed changes
Copilot reviewed 82 out of 94 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| toolchain/mfc/params/descriptions.py | Adds user-facing descriptions for new experimental/performance parameters. |
| toolchain/mfc/params/definitions.py | Registers new parameters (AMR, hybrid sensors, load-balance diagnostics) and target applicability. |
| toolchain/mfc/lint_docs.py | Treats new validator checks as non-physics doc checks. |
| tests/F980C769/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/ECABA006/golden-metadata.txt | Adds golden metadata for active-box test coverage. |
| tests/DD4CD8F3/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/CC4213FD/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/BD21A5C0/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/BCBA6E74/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/ACE05393/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/987D9025/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/852CCB81/golden-metadata.txt | Adds golden metadata for AMR-related golden tests. |
| tests/65C375B4/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/4DADE04B/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/454C565F/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/3A474BEE/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/2FC423D3/golden-metadata.txt | Adds golden metadata for a new/updated test. |
| tests/13945217/golden-metadata.txt | Adds golden metadata for moving-IB under AMR test coverage. |
| src/simulation/m_viscous.fpp | Clamps FD coefficient indexing to avoid ghost-region coefficient OOB in IB drag gradient evaluation. |
| src/simulation/m_time_steppers.fpp | Integrates active-box bounds into RK update loops and interleaves AMR fine-stage/subcycle operations. |
| src/simulation/m_start_up.fpp | Wires up new modules (rank timing, active-box, load-weight, SFC partition, AMR) into init/timestep/finalize and restart I/O. |
| src/simulation/m_sfc_partition.fpp | Adds analysis-only SFC tiling + weighted partition prediction and reporting. |
| src/simulation/m_rank_timing.fpp | Adds per-rank wall-time imbalance measurement helpers and reporting. |
| src/simulation/m_load_weight.fpp | Adds per-cell load-weight field construction and rank-level imbalance reporting. |
| src/simulation/m_hypoelastic.fpp | Refactors FD coefficient setup into a callable update routine (supporting AMR grid swaps). |
| src/simulation/m_global_parameters.fpp | Adds AMR working-state mirrors and slot selection helper plus defaults for new parameters. |
| src/simulation/m_data_output.fpp | Adds output/report hooks for load-weight, SFC partition, and rank-time diagnostics. |
| src/simulation/m_checker.fpp | Adds input validation/prohibits for active-box, hybrid sensors, load-balance, and AMR configurations. |
| src/simulation/m_active_box.fpp | Adds active-box initialization/growth and debug envelope checking. |
| src/simulation/m_acoustic_src.fpp | Adds AMR-aware handling of acoustic source support (bounding boxes and overlap abort). |
| src/post_process/m_start_up.fpp | Calls AMR fine-data reader and AMR overlay writer when amr is enabled. |
| src/post_process/m_global_parameters.fpp | Adds default-off amr flag for post_process overlay behavior. |
| src/post_process/m_data_output.fpp | Implements AMR fine-block overlay mesh/variables output (Silo/binary) and multimesh registration. |
| src/common/m_phase_change.fpp | Exposes per-cell Newton iteration count and threads it through relaxation to support load-weighting. |
| src/common/m_global_parameters_common.fpp | Adjusts start_idx lifecycle/allocation and makes load_weight_wrt visible to GPU macros. |
| src/common/m_derived_types.fpp | Introduces a simple t_box type used by new partitioning infrastructure. |
| src/common/m_box.fpp | Adds box/partition arithmetic helpers (equal/weighted splits, box-from-splits). |
| src/common/m_boundary_common.fpp | Skips BC buffer population during AMR fine advance to rely on coarse-driven ghost fill. |
| docs/module_categories.json | Registers new modules under documentation categories. |
| docs/documentation/readme.md | Adds AMR section link to the documentation index. |
| .typos.toml | Adds project-specific abbreviations to the spelling allowlist. |
| D = ((gs_min(lp) - 1.0_wp)*cvs(lp))/((gs_min(vp) - 1.0_wp)*cvs(vp)) | ||
|
|
||
| #ifdef MFC_SIMULATION | ||
| if (relax .and. load_weight_wrt) then |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1628 +/- ##
==========================================
- Coverage 61.67% 61.59% -0.09%
==========================================
Files 84 96 +12
Lines 21619 28741 +7122
Branches 3196 4705 +1509
==========================================
+ Hits 13334 17702 +4368
- Misses 6093 7909 +1816
- Partials 2192 3130 +938 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
Author
|
Upstream latent gap found during the MHD+AMR investigation (independent of this PR): Also for the record: MHD+AMR was attempted and re-gated on measured evidence rather than assumption — the coarse/fine seam is a continuous O(1) div(B) source that cleaning spreads but cannot remove (details in the amr.md support matrix row and commit ac203b1). |
sbryngelson
changed the title
Member
Author
|
Records the agreed target for the phase after the block-size cap work: give each refinement level its own box list AND its own rank mapping, with inter-level coupling by general point-to-point. Documents the measurement it rests on - cost tracks TOTAL box count near-linearly (exponent 0.68-0.93), and the derived cap shrinks with rank count so a fixed feature is tiled into more boxes the further you scale, which is why np=4 -> 8 regresses even though per-rank boxes fall. Explicitly retracts the intermediate conclusion that whole-box ownership must go. AMReX, Chombo and BoxLib all keep it; they scale on a small absolute cap, per-box scratch, and per-level distribution. The MFC ceiling was a cap DERIVED from the subdomain combined with solver scratch SIZED to the subdomain - amr_max_grid_size (3a71839) and idwbuff_alloc (7b1e493) respectively. Also records why the two alternatives were rejected (domain-aligned ownership surrenders load balance to the coarse cut; block sub-decomposition adds a second decomposition layer and keeps block-as-atom), what MFC already has pointing this way, and that Track 2's P2P parent<->child becomes the spine rather than one queue item - tower co-location then disappears as a consequence. Sequencing puts the m/n/p-keyed scratch conversion BEFORE relaxing the cap guard, since the other order is a silent out-of-bounds write, and requires the conservation ladder rather than field diffs for the many-to-many stages.
…factor (MFlowCode#1679) Three conflicts, resolved to keep both sides: - toolchain/mfc/params/definitions.py: upstream reordered the _nv(_SIM_POST) list (same name set as the merge base); our branch added five load-balance params. Resolution is the exact union. NAMELIST_VARS is a dict and every consumer sorts it, so registration order is not semantic. - src/post_process/m_global_parameters.fpp: keep amr = .false.; drop the mixture_err/alt_soundspeed defaults upstream moved into s_assign_common_defaults, which post already calls beforehand. - src/common/m_mpi_common.fpp: both sides rewrote s_mpi_decompose_computational_domain for orthogonal reasons. Ours replaced the inline rem_cells arithmetic with the box layer; upstream hoisted the MFC_POST_PROCESS/MFC_PRE_PROCESS policy blocks into s_apply_decomposition_policies. Take both: the box block supplies m/n/p and start_idx, the helper supplies silo offsets and local-domain adjustment, and rem_cells_by_dim(1) is set after the box block. Rather than reinstate our #ifdef MFC_SIMULATION for the AMR temperature ghost, use upstream's exchange_all_chemistry_temperatures flag (added in MFlowCode#1679 to keep stage guards out of src/common) and pass amr from simulation's call site. Same condition, no stage guard in common. Also fixes a silent semantic conflict git could not flag: upstream added a required third argument to s_initialize_eqn_idx, and the branch-only m_load_balance.fpp calls it. Passes six_eqn_alf_is_advected=.true. to match simulation's own call site, which that call deliberately duplicates. Verified: format clean, precheck 7/7, all three targets build, and the full golden suite passes 703/703 (69 AMR, 16 chemistry, all 8 ppn=2 tests) on AMD MI250X with amdflang OpenMP offload. The 32 convergence cases are the runner's default skip.
…loc's counterpart) Completes the allocation/runtime bound separation started in 7b1e493, which covered only the idwbuff-keyed sites. An idwbuff grep misses a whole family: m_riemann_solvers sizes on -1:m/-1:n/-1:p directly, m_weno on is*_weno (m-derived), and several modules on bare 0:m / -buff_size:buff_size+m. Adds m_alloc/n_alloc/p_alloc, derived by inverting idwbuff's own definition (end = m - beg), so they are identically m/n/p whenever idwbuff_alloc == idwbuff and 0 for a collapsed dim. Converted, inside @:ALLOCATE only: m_riemann_solvers 6, m_hypoelastic 8, m_bubbles_EL 8, m_rhs 8, m_bubbles_EE 5, m_body_forces 3, m_igr 2. Two things an idwbuff-only reading of the problem misses, both required and both invisible to every existing golden: - The x/y/z_cb, _cc and dx/dy/dz grid-coordinate arrays must be sized to the cap too. s_amr_swap_to_fine writes a block's own coordinates into these global arrays out to slot%m + buff_size (m_amr.fpp:3181 and the ghost loop below it), where slot%m is the block's FINE extent. The sw_* save buffers are whole-array assigned to/from them, so those move in lockstep or the shapes stop conforming. - m_weno gets allocation-only bounds (is1/is2/is3_weno_a) rather than a textual m -> m_alloc swap of is*_weno. s_compute_weno_coefficients is called WITH is*_weno and reads s_cb(i) => x_cb(i) across the range; widening that range would compute coefficients from cell-boundary coordinates that do not exist yet, and it divides by their differences. The allocations widen, the coefficient sweep does not, and AMR refills the tail per block (s_amr_recompute_weno_coefs -> m_amr.fpp:3432-3439). m_amr.fpp:463-467 already documents this same OOB class on the buff_size axis; that gate is untouched. Behaviourally a NO-OP: at the default amr_max_grid_size = 0, m_alloc == m and idwbuff_alloc == idwbuff identically, so every allocation is byte-for-byte as before. The amr_max_grid_size > fit_d guard is deliberately left in place - it is what still prevents a cap above the derived one, and relaxing it belongs with the test that can actually exercise this (goldens cannot: they all run the no-op path). Verified: format, precheck 7/7, simulation builds, 74/74 AMR + active_box + L0 goldens pass on MI250X/amdflang.
Removes the amr_max_grid_size > fit_d abort. Its own comment named the exit condition - 'needs the solver working set sized to the cap rather than to the subdomain, a separate increment' - and 8678224 did exactly that: idwbuff_alloc and m/n/p_alloc give the scratch amr_ref_ratio*amr_max_grid_size - 1 fine cells plus the ghost shell, so a block at the cap fits however small the subdomain becomes. This is what decouples the box set from the rank count. Previously the pinned cap could only ever be <= the derived one, so it could make blocks smaller and rank-invariant but never larger - and the derived cap shrinks as ranks are added, which is backwards for strong scaling. Also corrects three places that documented the old behaviour: the PHYSICS_DOCS text in case_validator.py, both copies of the AMR table in case.md, and the comment at m_amr.fpp:376 that justified the fine/coord array sizing by appealing to the abort being removed (it asserted no block exceeds the min-over-ranks local half-extent, which is no longer true when the cap is pinned). Derived-cap runs (amr_max_grid_size = 0) are untouched: they still take the min-over-ranks local half via fit_d. Verified on 2047x1023, amr_max_grid_size = 384, MI250X/amdflang under OMP_TARGET_OFFLOAD=MANDATORY. This configuration ABORTED before this commit (derived fit is 256 at np=4) and now runs clean: - np=4: 11 steps, no abort, no NaN - amr_maxc_fit = 384/384 and 14 boxes with IDENTICAL lo/hi at np=1 and np=4, i.e. the box set really is rank-independent - the coarse restart file is BITWISE identical between np=1 and np=4 - the fine restart file differs by exactly 504 bytes = the by-design per-rank ownership header (3*num_procs ints/block x 14 blocks: 168 - 42 = 126 ints), not physics - per-rank grid-keyed scratch grows 1.49x here (only y inflates; x already exceeded the cap). Cost is O(cap**num_dims) and INDEPENDENT of rank count. - 74/74 AMR + active_box + L0 goldens pass; precheck 7/7
…locks Track 2, steps 3a and 3b of docs/documentation/amr_per_level_distribution.md. Both were np=1-local and worked at np>=2 only because tower co-location pins a level-1 block and all its descendants to one rank. That co-location is the exascale blocker: it makes the balancer's smallest atom a whole tower of weight cost*rr**(l*d), so one deep tower concentrates work that adding ranks cannot relieve. These two commits remove the reason it exists. The enabler is s_amr_parent_foot: the parent-fine exchange box derived from REPLICATED metadata only (amr_region_*_all + the global amr_ref_ratio), so sender and receiver compute the same box with no handshake. amr_isect_lo/hi cannot serve that role - it is the empty footprint on a non-owner of the block. GATHER (parent -> child): the parent's owner packs its fine patch and sends; the block's owner unpacks into amr_cg. The pack/unpack use the same index map as the existing local s_amr_copy_parent_patch so the two sides cannot drift. The receive side is a separate routine taking only pblk - it must not name the parent field, because amr_slots(pblk) is unallocated on the receiving rank. RESTRICT (child -> parent): needed no new kernels. The child restricts LOCALLY and ships COARSE cells - rr**num_dims fewer values than shipping its fine block - which is correct because restriction is an overwrite, not an accumulate. Reuses s_amr_restrict_pack_device and s_l0_pack_unpack_block verbatim; the L0<->L1 scatter already documents their wire layout as compatible. Only the Cartesian path is needed: cyl_coord with amr_max_level > 1 is prohibited (m_checker.fpp:162). Both restrict call sites guarded on amr_rank_owns_block, which would have kept the PARENT's owner from posting its receive and deadlocked the pair. Opened to admit either participant; relax, reflux and rank timing stay behind the owner guard. Behaviour-preserving: when the two owners coincide - always at np=1, and everywhere today under co-location - both take the original local path, so the P2P branch is dormant and the existing goldens gate it. NOT yet done, and it is what blocks dropping co-location: s_amr_reflux_to_parent is a THIRD np=1-local path. Flipping co-location off without it drops cross-rank C/F reflux corrections SILENTLY - no crash, no deadlock, no np=1 golden failure, just conservation drift at np>=2. Also converts active_box's silent multi-rank self-disable into a declared @:PROHIBIT, so a production run cannot quietly get full-domain compute plus a warning line, and updates the two doc sites that described the old behaviour. Verified: format, precheck 7/7, simulation builds, 74/74 AMR + active_box + L0 goldens on MI250X/amdflang (gather and restrict each gated separately).
…lent disable Companion to 6832d29, whose message claimed these two sites were updated - they were staged from src/ only and missed. case.md and amr.md still said active_box 'disables itself with a warning' at num_procs > 1; it now aborts at input check (m_checker.fpp), so a production multi-rank run cannot quietly fall back to full-domain compute.
The Berger-Colella C/F correction from a level>=2 block into its parent was the last parent<->child path that assumed tower co-location. The parent's owner now applies it: it already holds the parent field and the parent-side creg, so only the child's freg crosses the wire, and only when the two owners differ. Under co-location they never do, so this is byte-identical to the previous owner-local form. s_amr_parent_foot moves to m_amr_registers (m_amr_registers cannot use m_amr) so the child-creg capture and the P2P gather/restrict/reflux share one copy of the parent-fine footprint formula. The capture now runs for every child of a block rather than only owned ones, since creg is the parent's own flux; framing it from replicated metadata instead of amr_isect_*_all, which is the empty sentinel for an unowned child. Precheck 7/7, 70/70 AMR goldens.
s_amr_advance_children drove one parent's whole subtree to completion before the next parent's, but the s_amr_fine_fine_halo(clev) interposed in its stage loop exchanges EVERY level-clev seam pair. A pair straddling two parents therefore had only one side present, and the side that was present read neighbour data from before the other parent's children were even set up. Tower co-location hid the MPI half (both ends of such a pair landed on one rank, so the exchange was a local device copy), which is why multi-level subcycling is checker-gated at np>1. Take a LEVEL rather than a parent slot and walk every block at that level together, looking each block's parent up per iteration. The level-1 driver's per-parent loop collapses to a single unconditional call, which also lets a rank owning a descendant but not the parent reach the fold. At np=1 this only re-orders independent per-parent work - each child reads solely its own parent's finished snapshots and its same-level neighbours - so results are unchanged. Precheck 7/7, 70/70 AMR goldens.
Step 4 of docs/documentation/amr_per_level_distribution.md. amr_fine_cut becomes one independent SFC cut per level, so every level is balanced across all ranks on its own weight; the twt tower rollup, the descendant-inherit pass, and f_amr_owner's level-1 anchor walk are deleted. A deep refinement tower no longer pins its whole subtree (weight cost*rr**(l*d)) to one rank, which was the granularity cap at depth. Per level rather than one mixed cut over all fine blocks: same-level boxes are disjoint and so carry distinct Morton keys, which the cut-point binary search needs. Mixed, a level-2 block sharing its parent's region_lo would collide with it. Splitting towers made four latent reads reachable, all of the same shape - a parent-slot field guarded on owning the CHILD, safe only while the two owners always coincided. s_set_amr_fine_geometry read amr_slots(pblk)%amr_ref_ratio (written by s_amr_alloc_slot, never called for pblk on a rank that does not own it, so undefined) and bisected amr_slots(pblk)%x_cb (unallocated there); the ghost-shell extension took lbound/ubound of the same unallocated array. Together they produced garbage cell widths and NaNs at the rank seam a few steps in - EF58E377, the one golden whose level-2 tiles actually divide across ranks. Footprint and refinement ratio now come from s_amr_parent_foot, and coordinates from a new s_amr_build_block_coords that replays the ancestor chain from the global L0 boundaries. Refinement is nested midpoint subdivision, so the replay reproduces exactly what bisecting the parent's stored array gave, and it collapses the level-1 and level>=2 coordinate branches into one call. The subcycle SETUP gather gets the same participant split the runtime gather already had. Precheck 7/7, 70/70 AMR goldens - the first run with a genuinely split tower, so the cross-rank gather/restrict/reflux are exercised rather than dormant.
…asured outcome The plan doc named load balancing once, only to reject an alternative - it had no mechanism, no metric, and no acceptance criteria, while the goal of the work is AMR *plus* load balancing. Adds a Load balancing section: the three mechanisms often conflated (L0 Cartesian rebalance, the cost model / imbalance diagnostic, and the AMR block assignment), the max/mean per-rank-weight metric with scale invariance as the requirement that actually matters, and the four structural limits that bind at scale - granularity floor, indivisible atom, staleness within a regrid interval, and cost-model blindness when load_weight_wrt is off. Notes that step 4 is the load-balancing enabler rather than only a distribution change: no assignment can fix an imbalance whose atom is a whole tower. Records the step-4 A/B honestly. Per-level distribution measured FLAT against co-location at np=1/2/4/8, and the result is not attributable because there is no imbalance metric - a flat end-to-end time cannot separate 'nothing to fix' from 'balancer did nothing'. Instrumentation therefore becomes step 5 and precedes further distribution work. Also records that the same case costs ~20x the coarse solve while its refinement accounts for ~2.5x, putting the dominant term in per-block launch overhead rather than balance. Marks steps 1-4 landed with commits, rewrites the stale Track 2 section, adds the parent-slot-guarded-on-the-child bug class that splitting towers exposed, and corrects the validation bar: cross-compiler coverage is CI's job, the local gate is amdflang OMP offload.
…s inconclusive The uniform control reaches only 18% parallel efficiency at np=8 (262k cells/rank, below what saturates an MI250X GCD), so both arms are latency-bound exactly where per-level distribution should matter most. The flat A/B therefore could not have detected the effect and is inconclusive, not negative. Adds the efficiency table and a standing rule: read the control's efficiency before trusting anything measured against it. Also corrects the overhead figure. It was quoted at np=8, where the uniform number is already degraded; at np=1, where both arms are compute-bound, AMR costs 40.7x the coarse solve for ~2.5x the nominal work - a ~16x residual, not ~8x.
The step-4 A/B measured flat at every rank count and could not be interpreted: with no imbalance metric a flat time cannot separate 'no imbalance to fix' from 'balancer did nothing' from 'gain offset by new P2P traffic'. Print the quantity the SFC cut is actually minimising - max_r W(r) / mean_r W(r) per level and in total - plus the count of ranks assigned nothing, which is the granularity floor showing up directly. Needs no MPI: wt, amr_block_level and amr_block_owner are replicated and identical on every rank (the cost vector is allreduced in s_amr_block_cost), so rank 0 prints. Gated behind load_weight_wrt, so every golden is unaffected. Precheck 7/7, 70/70 AMR goldens.
Audit of fc53e09 found two flaws in the report itself. (1) merge(mx/mean, 1._wp, mean > 0._wp) does not guard: merge is a function, so both arms are evaluated and the mean == 0 arm still divides by zero. Use a real if. (2) rw/tw were automatic arrays dimensioned 0:num_procs-1 - O(num_procs) on the STACK, in the one routine whose purpose is measuring behavior at large rank counts; the module already heap-allocates amr_owner_cut and amr_fine_cut at exactly that size. Allocate them. Also rename idle_ranks -> no_blocks_ranks / ranks_with_no_fine_block: the counter is per-LEVEL, and a rank with no block at a level still owns level-0 work, so the old name invited reading it as idleness. It is not; only m_rank_timing measures that. Separately, the s_amr_advance_fine_subcycle_all docstring claimed the L2-L2 seam halo was future work and that s_amr_check_seam_topology aborts on L2+ seams under subcycle - 3db24df removed that abort and the halo exists; and the s_amr_tile_box tsz docstring named a fixed amr_maxc_fit/2. Precheck 7/7, 71/71 AMR goldens.
The regrid child-box cap ran inside do lev = 2, amr_max_level but passed amr_maxc_fit/2 at every level. That constant is derived for level 2 specifically: a slot holds amr_ref_ratio*amr_maxc_fit fine cells (max_f1 = amr_ref_ratio*amr_maxc_fit - 1) and a level-lev block spans amr_ref_ratio**lev fine cells per coarse cell, so the cap must be amr_maxc_fit/amr_ref_ratio**(lev-1). At lev = 3 the fixed /2 admits a box twice what the slot holds. Identical at lev = 2 (amr_ref_ratio is 2 whenever amr_max_level > 1 - the checker gates rr /= 2 to single-level), so no existing golden moves, and 71/71 AMR goldens confirm that. NOT VERIFIED BY RUNNING: no golden covers amr_max_level >= 3, so this is arithmetic checked against the slot sizing, not observed behavior. It is strictly more correct than the constant it replaces and cannot regress level 2, but do not read the green suite as evidence that level 3 works. No call-site floor is needed - s_amr_tile_box already does tc = max(tc, 1) for exactly the zero case (collapsed dims pass 0 today).
The PROHIBIT's stated reason - that s_amr_advance_children advances level-2 children per-block with no L2-L2 halo - stopped being true at 3db24df, which removed the matching runtime abort saying 'the halo it warned about now exists'. 26e0d08 then extended the transpose across parents. Blast radius is narrow: every risky combination keeps its own gate (non-polytropic QBMM np>1, cyl_coord, IB np>1, moving IB, amr_ref_ratio /= 2, amr_max_blocks < 2), so this opens only plain Cartesian hydro at rr=2. No test expected the abort. New golden C45DBB52 covers it, VERIFIED by dumping block lo/hi/owner rather than assumed: adjacent L2 siblings [7,11]+[12,16] under one parent, and a child on a different rank from its parent (L2 [52,56] rank 0 under L1 [49,59] rank 1) at an intermediate regrid - the per-substep P2P SETUP gather, whose failure mode is a deadlock no tolerance check catches. NOT covered, deliberately: the L2 seam over MPI. Morton order keeps a contiguous run contiguous and the SFC cut is a single split point, so the adjacent pair lands on one rank; an eps x buf sweep (0.002-0.05 x 8,12) never split it, and contriving one would pin the golden to an SFC outcome any cost-model change would undo. Both ingredients are covered separately (level-2 fmul by A635AA56 at np=1, MPI seam transport by the single-level subcycle np=2 golden). Note cross-parent seams are impossible at any depth: the nesting window insets each parent by amr_cpat_mar >= 1. Precheck 7/7, 71/71 AMR goldens.
The granularity-floor section said a shallow hierarchy leaves ranks 'idle by construction', and I read the step-5 np=8 measurement the same way, reporting 'idle_ranks 6 of 8' as 75 percent of the machine idle. That is wrong. The counter is per-LEVEL: a rank holding no level-L block still owns level-0 work (level 0 covers every rank) and may hold blocks at other levels; even the TOTAL line means only 'owns no fine block'. So the number does not explain the 18 percent parallel efficiency - that still needs measured per-rank time, which is the point the metric section already makes. Also mark step 5 landed while stating that its m_rank_timing half, the half that measures idleness rather than inferring it, is not done.
Removing the multi-level subcycle PROHIBIT left twelve lines of explanation sitting in a list of checks with nothing after them - a comment justifying why something is NOT checked, in the file whose job is checking. The content is already in the two places it belongs: s_amr_advance_children's docstring (the level-wide walk and the interposed halo) and the C45DBB52 test comment (the amr_cpat_mar margin argument).
be94db3 shipped the per-level cap saying it was verified by reading the slot arithmetic, not by running, because no golden covers amr_max_level >= 3. It is now verified by running, and the evidence belongs where the next reader will be standing. m = 255, amr_max_level = 3, amr_buf = 48, np = 1: the fixed /2 keeps ONE oversized level-3 box where the per-level cap keeps 2, and the run aborts in s_amr_free_slot with a Fortran 'Invalid descriptor' and a core dump; the per-level cap exits 0. Two earlier counterfactuals produced byte-identical output and proved nothing - m = 63 was too small, and scaling the grid without scaling amr_buf left the boxes tracking the feature rather than the domain, far below either cap. A635AA56's comment already says it is the BUFFER that widens the box; applying that is what made the cap bind.
fahnab666
added a commit
to fahnab666/mfc-nabid
that referenced
this pull request
Aug 29, 2026
…isting lint debt Merges the 20 commits added to sbryngelson/up-mega since ee7758b: the CCE descriptor and GPU_DECLARE fixes for the Frontier AMR abort, the NVHPC declare-ordering fix, restart format v2 (per-block owner/extents), amr_blocking_factor defaulting to 4, and the level-order clustering reductions. The delta is confined to AMR: m_amr, m_amr_regrid, m_amr_restart, m_constants, m_global_parameters, cmake/GPU.cmake, 14 AMR goldens, and two AMR-only toolchain hunks. No JWL, IBM, Riemann, or EOS source file is touched and no non-AMR golden changes. Merge was conflict-free; the local static-AMR GPU allocation cap for IBM survives intact. Also clears lint debt that predates this merge and blocked any commit: formatter line-wrapping in six JWL/IBM files, the duplicated rxn_val PROHIBITs in pre_process/m_checker.fpp (already enforced in case_validator.check_stiffened_eos, as its own docstring states), the stale hyperelasticity row in case.md (removed upstream), the missing sec-mhd anchor, and m_jwl/m_jwl_sources missing from module_categories.json.
…ntier CCE gpu-acc) A declare-create on a module allocatable binds a present-table entry to the descriptor at program init; the move_alloc at the allocation site then swaps that descriptor out, so every later kernel lookup misses. All 36 AMR tests on the CCE gpu-acc lane died with 'find_in_present_table failed' for amr_cons_br (432 hits) and amr_scr_prim (9). amr_cg has always used move_alloc + GPU_ENTER_DATA with no declare and works on every lane; this makes the other two match. The declares were added for the lib-4425 uninitialized-descriptor abort, but the move_alloc is what actually fixed that, so they were redundant as well as harmful. amdflang's OpenMP runtime creates the mapping implicitly, which is why no local gate could see this.
s_populate_variables_buffers sits inside s_compute_rhs, which PH_COARSE wraps whole, so a uniform (amr=F) run reported 95% 'coarse' and no communication at all. That made the one comparison that matters impossible: how much of AMR's ~31% communication share is AMR's own versus the solver's baseline.
Halo probe counts distinct block slots this rank's metadata reads touch between regrids, reduced with MPI_MAX over ranks rather than reported from rank 0 (which owns a domain corner and is the least connected rank). The reduction is entered by EVERY rank: an earlier draft placed it inside the proc_rank == 0 guard, so one rank called ALLREDUCE while the rest ran ahead into other collectives and the job died with MPI_ERR_TRUNCATE. Grid efficiency reports tagged versus covered coarse cells, which prices amr_tag_eps and amr_buf and bounds the quoted payoff.
…reshold A histogram of the tagging ratio g/(2*r0) over ~49 regrids of this case shows the distribution is continuous from 0 to 0.5 with no gap, and amr_tag_eps = 0.05 already sits at a local minimum, so there is nowhere better to move it. Near the threshold the density is ~9,400 cells per unit ratio per regrid, so one flip across 49 regrids needs a perturbation of ~2e-6; roundoff is 1e-12 to 1e-14 and cannot reach that. The flipped tag is therefore a consequence of divergence, not its cause: roundoff is amplified by a chaotic configuration until it reaches ~1e-6, and only then do tags flip. t_step_stop 100 -> 25 keeps 12 regrids and the store still grows 25 -> 36 -> 49 blocks with the cap never binding, while giving roundoff far less room to grow. override_tol = 1e-11 covers the residual toolchain roundoff (CCE measured 1.06e-12) without being loose enough to hide a mesh divergence.
The allocation is guarded by hypo_nc_mode == hypo_nc_mode_dual_pass alone, but the teardown sat nested inside if (.not. igr), so any configuration with igr and dual_pass together leaked flux_n, flux_src_n and flux_gsrc_n along with their %sf payloads and device maps. Nothing couples igr to hypo_nc_mode in the validator, so the combination is reachable. Hoisted the teardown block out of the igr guard so the two are textually identical.
Nothing in the tree writes this array, but s_finalize_riemann_solver_hatR copies all of 1:sys_size out of it into flux_gsrc_n(id), which m_rhs folds into the RHS. Under cyl_coord with the dual-pass path that fed uninitialized memory into the solution. Mirrors the zeroing its twin flux_gsrc_rsx_vf already gets for the same reason.
s_compute_load_weight reads it on the first s_write_data_files, which for a run saving at t_step_start precedes any relaxation sweep, so the first load-weight diagnostic and load_weight.*.dat came from uninitialized memory. The device copy is the one that matters: the reader is a GPU_PARALLEL_LOOP and the only writer sits inside a device region, so a host-only assignment would have reached neither.
s_write_amr_restart marks format v2 with a NEGATIVE rank count, and the simulation reader was taught both formats, but the post_process reader was not. Its rank-count check therefore fires unconditionally (a negative can never equal num_procs) and reports a rank-count mismatch even when the counts match, which sends the reader after the wrong problem; it also still reads the 3*num_procs extents record that v2 does not write. Detect the negative marker in both the serial and MPI branches and abort saying what is actually wrong. This is a stopgap: reading v2 in post needs the whole-block ownership layout (one contiguous chunk per block, ownership from the file's 4-int record rather than derived geometrically), which is a separate change. Nothing in the default suite runs AMR with post_process as a target, which is why the break shipped unnoticed.
Only the parallel_io branch of s_write_amr_restart writes the negative rank count that marks format v2; the serial branch still writes a positive num_procs to p_all/p<rank>/<t_step>/amr_fine.dat. The guard added to the serial reader could therefore never fire, and its comment claimed a failure mode that path does not have. Verified against a real 8-rank AMR run: the parallel file's header is (-8, 224, 6), so the parallel guard is the one that matters.
The simulation path raises MFCException on a non-zero return code; the --test-all post_process path captured stdout and ignored the result entirely (common.system runs with check=False). post_process could abort, segfault or fail outright and the test still reported PASS as long as the simulation goldens matched. That is how a total break of the AMR post-process reader shipped unnoticed: --test-all ran post_process on every AMR case and threw the answer away. Verified both directions: an AMR test now fails where it previously passed, and non-AMR cases are unaffected.
Two defects, both in the AMR fine-block reader, both invisible until the test suite started checking post_process's exit code. First: format v2 was never taught to post. The writer marks v2 with a negative rank count, the simulation reader handles both formats, and post did not, so its rank-count check fired unconditionally (a negative can never equal num_procs) and blamed a rank-count mismatch on runs that used the right number of ranks. v2 stores one contiguous chunk per block written by that block's single owner, plus a 4-int (owner + 1, m, n, p) record. Ownership now comes from that record rather than from the reader's own geometric intersection, which also fixes a case where a rank overlapping a block it does not own would read a -1 extent and abort. The whole chunk is read and this rank's intersection sub-box copied out, so s_setup_amr_block still reconstructs coordinates from a valid local coarse index. v1 files stay readable. Second: post runs with a LARGER sys_size than the simulation for 5eq Lagrange bubbles, because it appends beta_idx as a post-only output slot. Comparing the file's conserved-variable count against post's inflated sys_size rejected every valid AMR + Lagrange-bubbles file. Size and read by the count the writer actually used.
Lines of Code
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An opt-in, default-off family of performance features and the measurement infrastructure they rest on. With all flags at their defaults the only touched production path is
s_mpi_decompose_computational_domain, refactored through the newm_boxmodule (byte-identical; covered by the existing suite).m_box(partition arithmetic),m_load_weight/load_weight_wrt(per-cell load-weight field + imbalance metric),m_sfc_partition/sfc_partition_wrt(Morton-SFC predicted-imbalance diagnostic),m_load_balance/load_balance(weighted static decomposition at init; AMR-fine-work-aware),m_rank_timing/rank_time_wrt(per-rank compute-time diagnostic).m_active_box/active_box: restricts reconstruction/Riemann/RK windows to a light-cone-grown box around non-ambient flow; strict-subset golden-tested.hybrid_wenoandhybrid_riemann(+hybrid_weno_eps,hybrid_smooth_flux): linear-optimal weights / central-or-Rusanov flux in smooth cells, full WENO/HLLC at flagged discontinuities (Jameson sensor, stencil-dilated, per-level under AMR).m_amr+m_amr_registers: two-level 2:1 refined block hierarchy; conservative restriction and conservative-linear prolongation with physics-specific closures; per-stage flux registers with Berger–Colella refluxing; Berger–Rigoutsos multi-block dynamic regrid; optional dt/2 subcycling; multi-rank (single-owner blocks assigned by Morton-SFC work balancing at each regrid, with migration; blocks may span rank seams via P2P coarse↔fine gather/scatter; same-level seam halo; distributed registers); restart (both IO modes, regridded-layout persistence); AMR-aware post-processing (fine blocks visualizable as Silo overlay domains); GPU-resident fine level on both OpenACC and OpenMP offload.Full algorithm and user documentation:
docs/documentation/amr.md(support matrix enforced at runtime by the checker — unsupported combinations abort with named messages, never silently).AMR physics support matrix (abridged; authoritative table in amr.md)
Supported and golden-tested: single- and multi-fluid (5-eq,
mpp_lim) · 6-eq with per-block pressure relaxation · viscous (refluxed) · phase change (relax) · chemistry incl. species diffusion · Euler–Euler bubbles (polytropic/non-polytropic, mono/polydisperse, QBMM incl. non-polytropic with per-blockpb/mvside-state; dynamic regrid + subcycle) · acoustic sources (coarse-grid support with regrid exclusion) · immersed boundaries (multi-body, static or prescribed-motion, incl. dynamic regrid with body-containment expansion and per-substage guards) · 2D axisymmetric (per-block WENO-coefficient recompute) · stretched grids (exact parent-bisection ghost coordinates + per-swap coefficient recompute) · hybrid WENO/Riemann sensors (per-level) · Lagrangian bubbles (cloud excluded from blocks; two-way coupling on the coarse grid; regrid clips around the moving cloud) ·active_box(blocks contained in the growing window; agrees with plain AMR to ~1e-14) · IGR (restriction-only coupling: fine sigma solve seeded/Dirichlet-bounded by the coarse solve; documented truncation-order seam, exact free-stream) · 1D MHD/RMHD (div(B)=0 by construction in 1D; HLL and HLLD, incl. relativistic).Gated with named aborts (documented rationale): surface tension (seam force imbalance is structural — three fixes attempted and diagnosed in amr.md) · 2D/3D MHD (attempted and measured: the c/f seam is a continuous O(1) div(B) source GLM cleaning cannot remove — needs constrained-transport-class B prolongation/reflux) · hyperelasticity · 3D cylindrical (global azimuthal filter) · force-driven IB (
moving_ibm=2) · STL bodies · Riemann-extrapolation BCs (bc=-4) ·amr_subcycleunder IGR · stretched grids with Lagrangian/IB-regrid (uniform-spacing index conversions).Validation evidence
Known issues (all non-gating or in progress)
continue-on-error): an intermittent post-detected NaN on the two Lagrangian+AMR goldens. Exhaustively unreproducible off GitHub's runners — the exact failing stack (NVHPC 24.3 SDK,-tp=px -Kieee, HPC-X MPI, and the CI docker image itself under apptainer) passes elsewhere, as do native/zen2 builds; 24.5+ green. Documented at the golden definitions.Review guide
The commit history is arc-ordered (active-box → load-weight → SFC → weighted decomposition → rank timing → hybrid → m_box → AMR rungs → physics envelope → CI/GPU hardening); reviewing by arc is much easier than by file. The AMR arc builds stepwise: static hierarchy → restriction/prolongation → fine advance → refluxing → regrid → subcycling → multi-rank → GPU → each physics rung with its own validation. Commit messages carry the validation evidence for their change (measured defects, golden UUIDs, repro details for CI fixes).
All parameters ship default-off with
case_validatorentries, runtime checker gates, andcase.md/amr.mddocumentation.