Skip to content

eos: guard null thermo in apply_primitive_limiter_#186

Merged
chengcli merged 2 commits into
mainfrom
eos-limiter-nullptr-fix
Jul 16, 2026
Merged

eos: guard null thermo in apply_primitive_limiter_#186
chengcli merged 2 commits into
mainfrom
eos-limiter-nullptr-fix

Conversation

@cshsgy

@cshsgy cshsgy commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • apply_conserved_limiter_ already guards options->thermo() before reading vapor/cloud species counts; apply_primitive_limiter_ did not, so any species-free EOS (e.g. ideal-gas) dereferenced a null thermo() every time the primitive limiter ran.
  • In release builds this doesn't throw — it silently corrupts the tracer slice of prim. Once a column's density/pressure get low enough (e.g. a strong transient updraft), the corrupted state reaches the implicit vertical solver as a singular Jacobian, which then spins retrying without bound.

How this was found

Reproduced on a cubed-sphere Jupiter case:

  • ideal-gas with limiter: false loses all density/pressure flooring entirely and crashes via a singular LU decomposition in the vertical implicit solver.
  • ideal-gas with limiter: true instead hits this null-thermo path and corrupts state (some columns pinned exactly at the density/pressure floor, others with velocities ~1e30+).
  • An identical run using ideal-moist with trace (~1e-10) vapor species — same forcing, same cold start, same perturbation — has no such bug (thermo() is non-null there) and survives the same event cleanly. This isolated the failure to the one line fixed here.

Confirmed the fix directly by constructing a low-density/high-momentum cell (the exact failure shape: tiny density, huge momentum, energy far too small for the resulting kinetic energy) and calling U->W on an ideal-gas block with limiter: true: before the fix this corrupts memory; after, density and pressure clamp cleanly to their floors as intended.

Test plan

  • pre-commit run passes on the changed file
  • snap_release builds clean
  • Direct unit-level repro: engineered conserved state (rho=1e-10, mom=1e6, E=1e5) through ideal-gas EOS compute("U->W", ...) with limiter: true — before: undefined behavior / memory corruption; after: rho and p clamp to their configured floors, no crash
  • Confirmed the bug does not reproduce under ideal-moist with trace species (same driver/config otherwise), isolating the fix to this code path

🤖 Generated with Claude Code

apply_conserved_limiter_ already checks options->thermo() before reading
vapor/cloud species counts; apply_primitive_limiter_ did not, so any
species-free EOS (e.g. ideal-gas) hit a null-thermo dereference every
time the primitive limiter ran. In release builds this doesn't throw --
it silently corrupts the tracer slice of prim with garbage, and once a
column's density/pressure are driven low enough (e.g. a strong transient
updraft), the corrupted state reaches the implicit vertical solver as a
singular Jacobian, which then spins retrying without bound.

Reproduced on a cubed-sphere Jupiter case: ideal-gas with limiter
disabled loses all density/pressure flooring and crashes via a singular
LU decomposition; enabling the limiter instead hits this null-thermo
path and corrupts state (some columns pinned exactly at the floor,
others with velocities on the order of 1e30+). An identical run using
ideal-moist with trace (~1e-10) vapor species -- same forcing, same
cold start, same perturbation -- has no such bug (thermo is non-null)
and survives the same event cleanly, isolating the failure to this one
line. Confirmed fixed by directly constructing a low-density/high-momentum
cell (the exact failure shape) and calling U->W: before the fix this
corrupts memory; after, density and pressure clamp to their floors as
intended.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a null-dereference/UB in the EOS primitive limiter path when running species-free EOS configurations (e.g., ideal-gas) by guarding access to options->thermo() before clamping tracer-related primitive entries. This aligns apply_primitive_limiter_ with the existing safety behavior already present in apply_conserved_limiter_, preventing silent state corruption that can propagate into the implicit vertical solver.

Changes:

  • Add a options->thermo() null guard around vapor/cloud tracer clamping in EquationOfStateImpl::apply_primitive_limiter_.
  • Document why the guard is required (species-free EOS types have unset thermo() and previously triggered UB).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cshsgy

cshsgy commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

Additional end-to-end confirmation on v2.7.5 (avoids an unrelated main-only regression in the cubed-sphere layout's yaml backend/device parsing that prevented a full run on tip-of-main): built this fix on a v2.7.5 worktree and re-ran the exact cubed-sphere Jupiter case that originally crashed.

Before the fix (same case, ideal-gas, limiter: true): pressure collapses geometrically column-by-column (p_min 2249 → 1936 → 1578 → 1205 → 860 → 612 Pa over cycles 1150–1175) until the corrupted state reaches the implicit solver.

After the fix, same forcing/IC/perturbation, same extreme event (v1max ~1509 m/s, same magnitude as before): p_min stays flat and healthy (8817 → 8814 → 8813 → 8816 Pa), rho_min stays flat (~0.0218), and all 6 ranks run cleanly past the previous crash point (cycle 1189) with no singular matrix and no corrupted state.

@chengcli chengcli merged commit 71f04ba into main Jul 16, 2026
3 checks passed
@chengcli chengcli deleted the eos-limiter-nullptr-fix branch July 16, 2026 03:08
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.

4 participants