Skip to content

Fix issues #250-264 - #267

Merged
asalmgren merged 11 commits into
AMReX-Fluids:developmentfrom
asalmgren:fix-issues-250-264
Sep 22, 2026
Merged

asalmgren merged 11 commits into
AMReX-Fluids:developmentfrom
asalmgren:fix-issues-250-264

Conversation

@asalmgren

@asalmgren asalmgren commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

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

Defensive gaps and diagnostics

Build coverage, docs and shipped decks

Testing

🤖 Generated with Claude Code

asalmgren and others added 9 commits May 18, 2026 09:38
Correctness and physics:

- AMReX-Fluids#250: delete the dead set_inflow_velocity / prob_set_inflow_velocity, which
  have had no caller since PR AMReX-Fluids#145 and wrote velocity component 2 with no
  AMREX_SPACEDIM guard for probtypes 33/333.
- AMReX-Fluids#251: refuse incflo.use_mac_phi_in_godunov, which only drops grad p from the
  MAC face prediction and is never read back as a pressure, and zero mac_phi in
  the LevelData constructor so the "macphi"/"error_mac_p" plot variables are
  defined on regridded and restarted levels.
- AMReX-Fluids#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.
- AMReX-Fluids#253: InitialPressureProjection passes set_inflow_bc = false; the field being
  projected is the body force (rho-rho0)/rho*g, not a velocity.
- AMReX-Fluids#254: ApplyCCProjection uses its own face scratch instead of the caller's
  u_mac/v_mac/w_mac, which the caller still needs for particle advection.
- AMReX-Fluids#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:

- AMReX-Fluids#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 abort message.
- AMReX-Fluids#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.
- AMReX-Fluids#258: an unrecognised incflo.geometry aborts instead of silently building a
  regular geometry; only an empty name or all_regular selects make_eb_regular.
- AMReX-Fluids#259: refuse a restart whose checkpoint prob_lo/prob_hi differ from the inputs
  the EB was built from.
- AMReX-Fluids#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:

- AMReX-Fluids#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 GNUmakefiles.
- AMReX-Fluids#261: drop the stale quarter-cell seeding note removed by PR AMReX-Fluids#213 and the
  tracer_particles_mass_density plot variable that nothing can produce.
- AMReX-Fluids#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).
- AMReX-Fluids#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.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
asalmgren and others added 2 commits September 21, 2026 16:13
…ent paths

incflo::ErrorEst and incflo::WritePlotVariables both dereferenced the result
of ParticleData::operator[] without checking it, and that operator returns
nullptr when no species of the given name exists.  With INCFLO_PARTICLES=ON
GCC inlines ParticleContainer::Increment into both call sites and reports
the potential null dereference, which fails the GCC NO EB 2D CI job now that
it builds particle support.

Look the container up once outside the level loop, abort if it is missing,
and use Increment rather than IncrementWithTotal in ErrorEst: the total was
discarded, so it was a global reduction per level for nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The GCC NO EB 2D job runs clang-tidy with --warnings-as-errors=*, and now
that it builds with particles the src/particles files are analysed for the
first time.  They are clean at the compiler level but tripped eight checks:

- clang-analyzer-optin.cplusplus.VirtualCall: both constructors called the
  virtual readInputs() during construction; qualify the calls so they are
  static, which is what actually happens anyway.
- readability-inconsistent-declaration-parameter-name: the MAC-velocity
  arguments of EvolveParticles/AdvectWithFlow and the box argument of
  initializeParticlesUniformDistributionInBox are named differently in the
  header and in the definition; the header now matches the definitions.
- bugprone-narrowing-conversions: make the Long -> int conversion of
  numPts() explicit.
- readability-qualified-auto, modernize-use-auto, modernize-use-bool-literals,
  readability-redundant-string-cstr, readability-redundant-control-flow and
  performance-unnecessary-copy-initialization: mechanical cleanups.

Also guard the two remaining unchecked dereferences of ParticleData's
operator[] in incflo_Tracers.cpp, matching the two fixed in the previous
commit, and make all three null branches end in a return so that neither
GCC nor the clang static analyzer treats the code after the Abort as
reachable with a null pointer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ApplyCCProjection uses the caller's u_mac/v_mac/w_mac as scratch, so evolveTracerParticles afterwards sees the projected new-time face field instead of the half-time MAC velocities InitialPressureProjection projects the body-force field (rho-rho0)/rho*g with set_inflow_bc=true, so ApplyNodalProjection writes the inflow *velocity* into its ghost cells (and runs enforceInOutSolvability on it) before taking the divergence compute_convective_term / compute_MAC_projected_velocities: with use_tensor_correction + Godunov + godunov_include_diff_in_forcing, the "viscous term" added to the edge-state forcing is divtau_o = (tensor - scalar) difference, not div(eta grad u)/rho incflo.use_mac_phi_in_godunov is half-implemented: it drops grad p from the MAC face prediction, solves into mac_phi and doubles it, but nothing ever reads mac_phi; the "macphi" plot variable is uninitialised memory on regridded or restarted levels set_inflow_velocity / prob_set_inflow_velocity have had no caller since PR #145, and the probtype 33/333 branches write velocity component 2 outside any AMREX_SPACEDIM==3 guard (out-of-bounds in a 2D build if ever revived)

2 participants