Repository navigation
fix issue 177 178 179 - #212
Merged
Merged
Conversation
jbbel
approved these changes
Aug 21, 2026
jbbel
approved these changes
Aug 21, 2026
This was referenced 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>
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.
No description provided.