Repository navigation
Conversation
jbbel
approved these changes
Aug 21, 2026
This was referenced Sep 16, 2026
Merged
asalmgren
added a commit
that referenced
this pull request
Sep 21, 2026
Fixes #240, Fixes #241, Fixes #242, Fixes #243, Fixes #244, Fixes #245, Fixes #246, Fixes #247, Fixes #248, Fixes #249. ## Medium severity - **#240** `ApplyCCProjection` allocated `cc_phi`/`cc_gphi` with `new` and never deleted them, leaking `1 + AMREX_SPACEDIM` cell-centred fields per call (device memory on GPU). This was reported as #191 and closed by PR #213 without the fix landing. They are now held in `Vector<MultiFab>` and handed to Hydro with `GetVecOfPtrs`, matching how `inv_rho`/`m_fluxes` are already done in the same function. - **#241** `DiffusionScalarOp` owned one EB solve op and one EB apply op, shared by the tracer and temperature passes. `MLEBABecLap` has no way to revert an EB Dirichlet BC back to homogeneous Neumann, so with only one of `eb_flow.tracer`/`eb_flow.temperature` given, the other field silently solved with a Dirichlet EB pinned to the first field's value and coefficient. The temperature now gets its own `m_eb_tem_solve_op`/`m_eb_tem_apply_op`, selected by an `is_temperature` flag passed from `diffuse_temperature`/`compute_laps_T`. Pure plumbing — the per-component logic is untouched. - **#242** `init_burggraf` evaluated the exact solution at `x-0.5, y-0.5` while the probtype-16 forcing, the lid profile and `DiffFromExact` all use `[0,1]^2`, so the IC was the exact solution translated by (-1/2,-1/2) — an O(1) slip velocity on no-slip walls. A regression from PR #160 (a cosmetic `Real()`-wrapping pass); the shift is removed. - **#243** The 2D cylinder cull in `incflo_PCInit` was self-contradictory: the assert admitted only `direction` 0 or 1, but the formula was the out-of-plane (direction 2) disk distance and ignored the direction it had just validated. Meanwhile `AdvectWithFlow` and `EB2::CylinderIF` treat 2 as a disk and 0/1 as slabs. So `direction = 2` aborted outright, and 0/1 seeded a disk while every later step confined particles to a slab. Init now uses the same convention as `AdvectWithFlow`. - **#244** A fresh start seeds tracer particles on level 0 only and nothing on that path ever calls `Redistribute()` (the restart path gets it from `ParticleContainer::Restart`). So initial `ErrorEst` counted zero particles on level >= 1 — with `refine_particles` as the criterion the initial hierarchy stopped at level 1 regardless of `amr.max_level` — and the first advection moved every particle under a fine patch with level-0 MAC velocities. Now redistributes after each level is created from scratch. ## Low severity - **#245** `steady_state = 1` was read and accepted, but `SteadyStateReached()` is an `amrex::Abort` stub with the real body `#if 0`'d. `Evolve()` masks its entry test with `&& !m_steady_state`, so the run paid for full initialization plus one complete time step and then died on what reads as an internal TODO (with the `stop_time` cap disabled for that step, and `steady_state_tol` silently ignored). Now refused at read time, exactly as PR #212 did for `amr.KE_int`. - **#246** `InitData` wrote `chk00000` without setting `m_last_chk`, and the restart branch left `m_last_chk = -1` while recording `m_last_plt = 0` instead of `m_nstep`. When the loop then takes no steps, Evolve's final-output block rewrote the step's checkpoint and plotfile — `UtilCreateCleanDirectory` renames the existing directory to `*.old.*` first, so the very checkpoint the run restarted from was renamed and replaced by an identical copy. Both counters now record the step of each write. - **#247** `ReadCheckpointFile` took `finest_level` from the header and looped `MakeNewLevelFromScratch` over it, but `grids`/`dmap`/`m_leveldata`/`m_t_new` are all sized `max_level + 1` from the *current* inputs. Restarting with a smaller `amr.max_level` wrote past the end of those vectors — silent heap corruption in a release build. Now aborts with a diagnostic naming both values. - **#248** The `StepType::Corrector` branch of `update_temperature` passed `get_laps_new()` (the tracer Laplacian, `m_ntrac` components) to `compute_laps_T` instead of `get_laps_tem_new()`. Unreachable today — `ReadParameters` refuses `use_temperature` with `advection_type = MOL` — but it would clobber the tracer's `laps` and alias out-of-bounds components for `ntrac > 1` the moment that guard is lifted. One-token fix. - **#249** `update_density` used `ng = 0` in the corrector, so the ghost cells of `density`/`density_nph` still held predictor values. `ApplyCCProjection` averages `density_nph` to faces via `amrex_avg_cc_to_fc`, which reads the ghost cell on the low face of every grid, so the `dt/rho` face coefficient was assembled from two different time levels on faces coinciding with grid boundaries — making MOL + `use_cc_proj` + variable-density runs depend on `amr.max_grid_size`. Now one ghost cell in both passes, so the corrector's `fillpatch_density` refreshes them. ## Testing Built clean with no warnings in four configurations: `test_2d` (2D EB), `test_no_eb_2d` (2D no-EB), `test_3d` (3D EB), `test_no_eb_3d` (3D no-EB). Note that **#242** (burggraf IC) and **#249** (corrector density ghost cells) change numerical output, so the affected benchmarks will need regolding. `benchmark.burggraf` in particular: the deck runs to `stop_time = 5` at Re = 100, so the `Norm0` printed by `test_convergence_burggraf` was previously measuring the decay of the spurious transient rather than the discretisation error of the steady solution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
asalmgren
added a commit
that referenced
this pull request
Sep 22, 2026
Fixes #250, fixes #251, fixes #252, fixes #253, fixes #254, fixes #255, fixes #256, fixes #257, fixes #258, fixes#259, fixes #260, fixes #261, fixes #262, fixes #263, fixes #264. ## Correctness / physics - **#250** Delete the dead `set_inflow_velocity` / `prob_set_inflow_velocity`, which have had no caller since PR #145 and wrote velocity component 2 with no `AMREX_SPACEDIM` guard for probtypes 33/333. - **#251** Refuse `incflo.use_mac_phi_in_godunov`: it only drops grad p from the MAC face prediction and is never read back as a pressure. Also zero `mac_phi` in the `LevelData` constructor so the `macphi` / `error_mac_p` plot variables are defined on levels created by regrid or restart. - **#252** `use_tensor_correction` now forces `godunov_include_diff_in_forcing` off, because `divtau_o` then holds the (tensor - scalar) difference rather than the full explicit viscous term the edge-state forcing expects. - **#253** `InitialPressureProjection` passes `set_inflow_bc = false`; the field being projected is the body force `(rho-rho0)/rho*g`, not a velocity. - **#254** `ApplyCCProjection` uses its own face scratch instead of the caller's `u_mac/v_mac/w_mac`, which the caller still needs for tracer particle advection. - **#256** Refuse probtypes 1100/1102 in a 2D build and guard the z-direction code in `init_jump`, which otherwise wrote past the end of a 2-component FAB. ## Defensive gaps and diagnostics - **#255** Reject a mixed BC combined with `use_tensor_solve` at read time (neither `MLTensorOp` nor `MLEBTensorOp` implements Robin BCs), add the `BC::mixed` case to `get_diffuse_tensor_bc`, and fix `get_diffuse_velocity_bc`'s copy-pasted abort message. - **#257** Every EB builder now requires `max_level` coarsenings, as the STL builder already did, so coarse AMR levels get EB data in their domain ghost cells. - **#258** An unrecognised `incflo.geometry` aborts instead of silently building a regular geometry; only an empty name or `all_regular` selects `make_eb_regular`. - **#259** Refuse a restart whose checkpoint `prob_lo`/`prob_hi` differ from the inputs the EB was built from. - **#264** Refuse the unimplemented `divu` plot variable when the inputs are read rather than at the first plotfile, and refuse a non-zero `incflo.ic_p`, which only sets `m_p000` and would otherwise be silently dropped. ## Build coverage, docs and shipped decks - **#260** Build particle support in one GCC and one CUDA CI job, run the 2D smoke test with tracer particles, and add `USE_PARTICLES` to the four `GNUmakefile`s. - **#261** Drop the stale quarter-cell seeding note removed by PR #213 and the `tracer_particles_mass_density` plot variable that nothing can produce. - **#262** Document the input keys the code actually queries (`ro_0`, `incflo.write_eb_surface`, `amr.refine_particles`, `mg_rtol`/`mg_atol`, the `scalar_diffusion`/`tensor_diffusion` prefixes and the `mg_` iteration keys). - **#263** Rename `incflo.v`, `amr.plot_p` and `incflo.use_godunov` in the shipped decks, drop the unread `cylinder.height`, and mark `cylinder.speed` (and the two decks it was the only flow driver for) as not implemented. ## Testing - 2D/3D x EB/no-EB x with/without particles: five configurations, all build with no errors or warnings. - Every new guard confirmed to fire at runtime: `use_mac_phi_in_godunov`, `ic_p`, `divu` (all three entry paths), 2D probtype 1100, typo'd and 3D-only geometry names, mixed BC + tensor solve, and the EB restart domain mismatch. - Runs clean: `benchmark.split` (mixed BC, `max_level = 1`), the `eb_chkptfile` write -> read round trip, a 3-level `rayleigh_taylor` with `macphi` under `amrex.init_snan=1`, `use_cc_proj = 1` with tracer particles, and a sweep of shipped 2D/3D decks. - One pre-existing failure, unrelated to this branch: `test_3d/inputs.channel_sphere` aborts with `MLMG failed.` on unmodified source too (and its `amr.max_level = 0`, so the #257 change is a no-op there). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
closes #171
closes #176
closes #181
closes #188
closes #189
closes #191