Skip to content

fix issues 117 118 119 120 121 122 - #124

Merged
asalmgren merged 1 commit into
AMReX-Fluids:developmentfrom
asalmgren:fix-issues-117-122
Sep 14, 2026
Merged

asalmgren merged 1 commit into
AMReX-Fluids:developmentfrom
asalmgren:fix-issues-117-122

Conversation

@asalmgren

Copy link
Copy Markdown
Contributor

Fixes #117, fixes #118, fixes #119, fixes #120, fixes #121, fixes #122.

#117 — ppm_trace_sources accepted silently for ppm_type = 1 and never used

The PPM source-tracing block in PPM.cpp is commented out and PLM never had one (trace_ppm takes srcQ as an unnamed unused parameter), so the sources are added in the transverse step for both reconstructions. The warning-and-reset now fires for every ppm_type, not just ppm_type = 0 — whose message implied tracing worked for ppm_type = 1. The option is also documented in _cpp_parameters.

#118 — numGrow is one ghost short for flatten_eb in EB Godunov + redistribution

The EB Godunov traces on grow(bx,4) (FluxRedist) / grow(bx,5) (StateRedist), and flatten_eb needs q at ±3 around each traced cell. With the old counts it hit the !q.contains(...) early exit on the outer ring and returned 1, silently dropping the outward flattening component — and through redistribution those states reach the update of valid cells, so results depended on the box layout. Godunov FluxRedist 6 → 7, StateRedist 7 → 8. NoRedist (which only needs traced states within distance 2) and every MOL case are unchanged, so the FluxRedist || NoRedist branch is split.

#119 — eb_weights_type = 1 was not the total energy

The weight was rho*(rho*e + |u|^2/2): the expression came from CNS, where q(QEINT) is the specific internal energy, but CAMR's QREINT is rho*e (Hydro_ctoprim.H:89). It is now the actual total energy density rho*e + 0.5*rho*|u|^2. Default is eb_weights_type = 2, so no shipped input changes.

#120 — slopetmp freed while the Riemann kernels may still read it

MOL_umeth_eb allocated its slope scratch on the default arena, whose free is immediate, and returns without synchronizing. It now uses The_Async_Arena() like every other per-box scratch FAB in the EB hydro path. Preventive: no wrong answer is reachable at the current commit.

#121 — regenerating CAMR_params.H broke the WARN_ERROR builds

The checked-in header carries [[maybe_unused]] on all 38 declarations, but the generator did not emit it, so following the header's own instructions (edit _cpp_parameters, run mk_params.sh) produced a tree that fails every CI build. parse_CAMR_params.py now emits the attribute; mk_params.sh reproduces all three headers byte-identically (verified — regenerating leaves no diff).

Additionally the file-scope #include "CAMR_params.H" is dropped from the five Exec/*/prob.H. None of them, nor any translation unit that includes them, references a parameter; it was giving four translation units 38 unused file-scope statics and made a prob.H reference to e.g. small_dens bind to a zero-initialized copy instead of CAMR::small_dens.

#122 — converging nozzle silently built an unrelated geometry in 2D

LatheIF revolves about the origin in 2D and RotationIF ignores its axis argument there, so CAMR.geometry = converging-nozzle produced nonsense with no diagnostic. It now aborts outside DIM = 3, and converging_nozzle.inp records that it is a 3D problem while this directory's GNUmakefile defaults to DIM = 2.

Also

Removes an unused local in MovingPlane_Shock/prob.H that made that directory fail a WARN_ALL/WARN_ERROR build. Pre-existing and unrelated — CI builds only Sod and SodPlusSphere, so it had never been compiled with those flags.

Testing

Built every Exec directory with WARN_ALL=TRUE WARN_ERROR=TRUE:

Directory Config Result
Exec/Sod 2D, non-EB SUCCESS
Exec/SodPlusSphere 2D, EB SUCCESS
Exec/SodPlusSphere 3D, EB SUCCESS
Exec/DoubleRamp 2D, EB SUCCESS
Exec/ReReTest 2D, EB SUCCESS
Exec/MovingEBCases/MovingPlane_Shock 3D, moving EB SUCCESS

Ran Sod; SodPlusSphere across all four do_mol x FluxRedist/StateRedist combinations (the paths whose numGrow() changed), energy conserved in each; DoubleRamp; ReReTest; and MovingPlane_Shock.

Layout check: SodPlusSphere EB Godunov + FluxRedist, 10 steps, hydro_tile_size 1024x16 vs 8x8 — fcompare gives relative errors <= 3e-15 on all fields. Note fcompare refuses differing BoxArrays, so this varies tiling rather than max_grid_size, and this input may not place a shock near a cut cell in the way #118 describes; it is a consistency check, not a reproduction of the original discrepancy.

🤖 Generated with Claude Code

- AMReX-Fluids#117 ppm_trace_sources is not implemented for any ppm_type: the PPM
  source-tracing block is commented out and PLM never had one, so the
  sources are added in the transverse step either way.  Warn and reset
  for every ppm_type instead of only for ppm_type = 0, whose warning
  text implied it worked for ppm_type = 1.  Also documents the option
  in _cpp_parameters.

- AMReX-Fluids#118 numGrow (EB Godunov + redistribution) was one ghost short for
  flatten_eb: the EB Godunov traces on grow(bx,4)/grow(bx,5) and
  flatten_eb needs q at +-3 around those cells, so on the outer ring it
  took the !q.contains() early exit and dropped the outward flattening
  component -- and those states feed the redistributed update, making
  the answer depend on the box layout.  FluxRedist 6 -> 7, StateRedist
  7 -> 8 for Godunov.  NoRedist (which only needs traced states within
  distance 2) and all MOL cases are unchanged.

- AMReX-Fluids#119 eb_compute_div: the eb_weights_type = 1 "total energy" weight was
  rho*(rho*e + |u|^2/2) because CAMR's QREINT is rho*e, not the specific
  internal energy of the CNS code this was ported from.  Use the actual
  total energy density.

- AMReX-Fluids#120 MOL_umeth_eb: allocate slopetmp on The_Async_Arena like the rest
  of the EB scratch, so it is not freed while the Riemann kernels
  reading it may still be in flight.  Preventive; no wrong answer is
  reachable today.

- AMReX-Fluids#121 parse_CAMR_params.py now emits the [[maybe_unused]] the
  checked-in CAMR_params.H carries, so re-running mk_params.sh no longer
  produces a header that fails the WARN_ERROR CI builds.  mk_params.sh
  now reproduces all three headers byte-identically.  Also drops the
  file-scope #include "CAMR_params.H" from the five Exec/*/prob.H, which
  none of them needs and which made a prob.H reference to e.g.
  small_dens bind to a zero-initialized copy rather than CAMR::small_dens.

- AMReX-Fluids#122 make_eb_converging_nozzle aborts outside DIM = 3: in 2D amrex's
  LatheIF revolves about the origin and RotationIF ignores the axis
  argument, so it silently built an unrelated geometry.  Notes in
  converging_nozzle.inp that it is a 3D problem.

Also removes an unused local in MovingPlane_Shock/prob.H that made that
directory fail a WARN_ALL/WARN_ERROR build (pre-existing; CI builds only
Sod and SodPlusSphere).

Built every Exec directory with WARN_ALL=TRUE WARN_ERROR=TRUE (Sod,
SodPlusSphere 2D and 3D, DoubleRamp, ReReTest, MovingPlane_Shock) and ran
Sod, SodPlusSphere (all four do_mol x FluxRedist/StateRedist
combinations), DoubleRamp, ReReTest and MovingPlane_Shock.  SodPlusSphere
EB Godunov agrees to roundoff between hydro_tile_size 1024x16 and 8x8.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@asalmgren
asalmgren merged commit e082e76 into AMReX-Fluids:development Sep 14, 2026
2 checks passed
@asalmgren
asalmgren deleted the fix-issues-117-122 branch September 14, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment