CAMR_reset_internal_e: UEDEN correction at line 117 is always zero (UEINT written first)

[WeiqunZhang]

Severity: high/medium · Category: physics-numerics · Fix order: 7 of 25 — fix this 7th.

_{Filenames are numbered in reverse fix order: 001 = fix last, 025 = fix first. This file is 019.}

Location: Source/Utils/CAMR_reset_internal_e.H:117

Based on commit 8f87fa8 (line numbers refer to that tree).

The five findings below are independent reports of the same statement-ordering bug in the allow_negative_energy == 0 branch; one swap fixes all of them.

The defect

What is wrong — UEDEN correction is a guaranteed no-op: UEINT is overwritten with rho*e_out at line 114 before line 117 computes UEDEN += (rho*e_out - S(i,j,k,UEINT)), which is identically zero; the twin branch at lines 84-90 updates UEDEN before overwriting UEINT. _{Reported independently at this same line by F042, F050, F052, F011; each reviewer's own wording and evidence is under Verification evidence below.}

Why it matters

Default configuration (allow_small_energy=1, allow_negative_energy=0, dual_energy_update_E_from_e=1): a cell with UEDEN>=0 but UEINT<=0 after a hydro update gets UEINT reset from small_temp, but UEDEN is never adjusted, leaving E inconsistent with e+K and silently ignoring the dual_energy_update_E_from_e request.

How to reach it

• Default build/params (allow_small_energy=1, allow_negative_energy=0, update_E_from_e=1, eta2=1e-4; CAMR_defaults.H:23-28), any shipped deck (e.g. 2D EB DoubleRamp shock); needs only a kinetic-dominated cell with UEINT<=0, UEDEN>=0. Branch executes; alleged damage does not.

Note from the audit

F039: F011 folds into this entry, and it is the audit's only finding on which the two lenses disagreed — the single PLAUSIBLE verdict referenced in the summary table. Its lens-2 refutation is quoted below; in short, the inconsistent UEDEN this branch leaves behind has no consumer at default settings, because dual_energy_eta1 defaults to 1.0 (CAMR_defaults.H:24) and Hydro_ctoprim.H:59 therefore always takes QREINT from UEINT. F039's own defect — the UEDEN correction being a guaranteed no-op — stands independently of that.

Suggested fix

Take the delta against the pre-floor (rho e): stash Real rhoe_old = S(i,j,k,UEINT); before the overwrite rather than relying on statement order, so the intent survives later edits. The allow_small_energy == 0 twin at lines 87-90 is the same algorithm written correctly and is the authority here; git history holds only the initial import (0656702) and a header fix (5e49d6c), so this has been wrong since the C++ kernel landed. Honoring dual_energy_update_E_from_e = 1 is the only way E learns about the floor, and UEDEN is read directly by reflux, average-down and the eint1 derive (Source/Utils/Derive.cpp:217), not only through Hydro_ctoprim.H. It also lifts UEDEN strictly positive, protecting ctoprim's AMREX_ALWAYS_ASSERT(u(i,j,k,UEDEN) > 0.) and the divide at Hydro_ctoprim.H:59. Maintainer calls: whether to tally the energy the floor adds, and whether the dual_energy_eta1 = 1.0 default should stand. No other fix needs to land with this.

For Source/Utils/CAMR_reset_internal_e.H:117 (F011):

--- a/Source/Utils/CAMR_reset_internal_e.H
+++ b/Source/Utils/CAMR_reset_internal_e.H
@@ -111,10 +111,10 @@
 
             } else if (S(i,j,k,UEINT) <= 0.) {
                 EOS::RTY2E(rho,small_T,massfrac,e_out);
-                S(i, j, k, UEINT) = rho * e_out;
 
                 if (l_dual_energy_update_E_from_e == 1) {
                     S(i, j, k, UEDEN) += (rho * e_out - S(i,j,k,UEINT));
                 }
+                S(i, j, k, UEINT) = rho * e_out;
             }
         }

_{Moves the UEINT overwrite after the UEDEN increment so the increment sees the old (non-positive) rhoe, exactly mirroring the allow_small_energy==0 twin at lines 87-90. Pure reordering: no new variables, same EOS::RTY2E call. Maintainer should confirm the intended twin convention is 'add the delta to E' rather than 'E = rhoe + rho*ke' as used in the UEDEN<0 branch at line 101. (sketch written against F039)}

_{Diff(s) are against 8f87fa8, written from the current source and verified only with git apply --check — never compiled, never run, never applied to the tree. Treat them as precise intent, not tested patches.}

Verification evidence

F011 — plausible — verifiers disagreed, see caveat below

Reported as: In the allow_negative_energy==0 branch, when UEINT<=0 the code sets S(UEINT)=rhoe_out at line 114 BEFORE the dual-energy correction at line 117, so 'S(UEDEN) += (rhoe_out - S(UEINT))' always adds exactly zero and UEDEN is never corrected; the sibling branch at lines 84-91 does it in the correct order.

Failure scenario: Default configuration (allow_small_energy=1, allow_negative_energy=0, dual_energy_update_E_from_e=1): in a high-Mach/strong-rarefaction cell with UEDEN>=0, UEINT<=0, and rho_eint=UEDEN-rhoke failing the eta2 test, UEINT is floored to rhoe(small_temp) but UEDEN is silently left unchanged, leaving UEDEN < UEINT + rho*ke permanently inconsistent and re-triggering floors every step.

Lens 1 (refutation attempt): Line 114 S(i,j,k,UEINT) = rho * e_out; precedes line 117 S(i,j,k,UEDEN) += (rho*e_out - S(i,j,k,UEINT)); -> increment is identically 0. Twin branch lines 87-90 updates UEDEN before overwriting UEINT. Defaults (CAMR_defaults.H: allow_small_energy=1, allow_negative_energy=0, dual_energy_update_E_from_e=1) make the branch reachable via clean_state->computeTemp->reset_internal_energy. However 'permanently inconsistent, re-triggering floors every step' is overstated: the next reset call takes lines 109-110 (UEINT>0 now) and sets UEDEN=UEINT+rho*ke, restoring consistency after one step.

Lens 2 (reachability/intent): Line 117's += is truly a no-op (dead code, unchanged since import 0656702), but the alleged failure is masked: after the floor UEINT>0, so the next reset call takes lines 109-110 (UEDEN = UEINT + rho*ke), healing UEDEN before any consumer — clean_state runs at CAMR_advance.cpp:83 and via expand_state/FillPatch (CAMR.cpp:1249) ahead of hydro. Meanwhile nothing reads UEDEN: default eta1=1.0 makes ctoprim always take QREINT from UEINT (Hydro_ctoprim.H:59-62); Timestep.H:38 uses UEINT. Floors do not re-trigger; inconsistency lasts one reset cycle.

F039 — confirmed (one verifier lens)

Lens 1 (refutation attempt): Confirmed: S(i,j,k,UEINT) = rho * e_out; (line 114) then S(i,j,k,UEDEN) += (rho*e_out - S(i,j,k,UEINT)); (line 117) is += 0. Sibling branch lines 87-90 shows intended order (UEDEN increment before UEINT overwrite). Default parameters route here; no fix in git history (only initial import + header-fix commits touch this file). E stays inconsistent with e+ke for that step; dual_energy_update_E_from_e request is a silent no-op in this branch.

F042 — confirmed (one verifier lens)

Reported as: In the default dual-energy branch (allow_small_energy=1, allow_negative_energy=0), UEDEN is incremented by (rhoe_out - S(UEINT)) AFTER line 114 already set S(UEINT)=rhoe_out, so the increment is exactly zero and UEDEN is never corrected.

Failure scenario: Default-parameter run, kinetic-energy-dominated cell where the update leaves UEINT<=0 while UEDEN>=0 (strong shock/rarefaction, common in EB small cells): UEINT is floored to rho*e_small but UEDEN keeps its inconsistent value, silently breaking the dual-energy bookkeeping that dual_energy_update_E_from_e=1 requests; sibling branch at lines 87-90 shows the correct read-before-write order.

Lens 1 (refutation attempt): Same defect confirmed: line 114 overwrites UEINT with rho*e_out before line 117 computes the delta against it, so UEDEN += 0. Branch reachable under defaults (allow_small_energy=1, allow_negative_energy=0, dual_energy_update_E_from_e=1) for kinetic-dominated cells where UEDEN>=0 but UEINT<=0 — plausible in EB small cells after redistribution; clean_state is called on both crse/fine after EB updates (CAMR.cpp:699,710). May relate to open issue #70 (CAMR vs CNS_EB behavioral differences in strong-shock EB runs). Note: consistency is restored one call later via lines 109-110.

F050 — confirmed (one verifier lens)

Reported as: In the default branch (allow_small_energy=1, allow_negative_energy=0), UEINT is overwritten with rhoe_out at line 114 before line 117 computes the total-energy correction 'S(UEDEN) += (rhoe_out - S(UEINT))', which is therefore always += 0; UEDEN never receives the intended update (contrast the correct ordering at lines 87-90).

Failure scenario: Default parameters (allow_small_energy=1, allow_negative_energy=0, dual_energy_update_E_from_e=1): any cell with UEDEN >= 0 but UEINT <= 0 (kinetic-energy-dominated flow after the hydro update, via clean_state -> reset_internal_energy) gets UEINT floored but UEDEN left unchanged, so E != e + ke persists and the dual-energy states stay inconsistent.

Lens 1 (refutation attempt): Verified in current source: S(i,j,k,UEINT) = rho * e_out; then S(i,j,k,UEDEN) += (rho*e_out - S(i,j,k,UEINT)); — same-thread read of the just-written value, no aliasing subtlety; the increment is exactly zero. Correct ordering exists at lines 87-90 of the allow_small_energy==0 twin. Path exercised by default config via CAMR::reset_internal_energy (CAMR.cpp:1130) called from computeTemp/clean_state after every hydro update.

F052 — confirmed (one verifier lens)

Reported as: In the allow_negative_energy==0 / UEINT<=0 branch, UEINT is overwritten with rhoe_out before UEDEN is incremented by (rhoe_out - UEINT), so the increment is identically zero and UEDEN is never updated, unlike the sibling branch at lines 84-90 which updates UEDEN first.

Failure scenario: Default options (allow_small_energy=1, allow_negative_energy=0, dual_energy_update_E_from_e=1): a kinetically dominated cell with UEDEN>=0, UEINT<=0 and (UEDEN-rhoke)/UEDEN<=eta2. UEINT is floored to rhoe(small_temp) but UEDEN keeps only kinetic energy, so E and e stay inconsistent and every reset re-floors; the E-from-e update option silently does nothing here.

Lens 1 (refutation attempt): Confirmed no-op: line 114 writes rhoe_out into UEINT, line 117 adds (rhoe_out - UEINT)=0 to UEDEN; twin at lines 84-91 orders the operations correctly. Reachable with default keys. One correction to the alleged failure: 'every reset re-floors' is wrong — after the floor UEINT>0, so the next reset call hits lines 109-110 and rewrites UEDEN=UEINT+rho*ke, so the floor does not re-trigger and E/e consistency returns after one step. The core defect (UEDEN not updated when requested at this call) stands.

Caveat: at least one finding here is plausible, not confirmed — the two verifier lenses disagreed. The premise was upheld but the consequence could not be established. Confirm before investing in a fix.

---

_{Based on commit 8f87fa8, which is also the tree the audit verified against. From an automated audit of Source/, Tutorials/ and Util/. Audit finding ids: F011, F039, F042, F050, F052. Reviewer unit(s): Utils-misc, theme:2d-3d-parity, theme:eb-smallcell, theme:gpu-capture, theme:state-comp-indexing. Nothing here was compiled or run — the failure scenarios are code reasoning, so the reaching configuration above is the cheapest way to confirm or refute it.}

[asalmgren]

closed by #110