ZeroingOutForPlotting zeroes all negative plotfile values (signed < 1e-12 test, no else)

[WeiqunZhang]

Severity: medium · Category: correctness · Fix order: 11 of 25 — fix this 11th.

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

Location: Source/CAMR.cpp:1291

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

Both findings are the same block at Source/CAMR.cpp:1289-1295, reported from two angles: the missing magnitude test, and the missing else.

The defect

What is wrong — ZeroingOutForPlotting zeroes every component whose value is less than 1e-12 -- including all legitimately negative values (momenta, velocities) -- because the threshold test is missing std::abs. _{Reported independently at this same line by F044; each reviewer's own wording and evidence is under Verification evidence below.}

Why it matters

CAMR_USE_MOVING_EB build, any flow with a negative velocity component (e.g. leftward-moving shock): writePlotFile (CAMR_io.cpp:584) runs this on plotMF, so every plotfile shows 0 for UMX/UMY/UMZ wherever momentum is negative, silently corrupting all visualization/post-processing of moving-EB runs.

Suggested fix

Make the second block an else branch and compare magnitudes, so only denormal-scale noise in uncovered cells is cleaned. The covered-cell branch is already redundant: CAMR_io.cpp:580 calls amrex::EB_set_covered(plotMF,0.0), which zeroes exactly the same cells (flagarr.isCovered(), i.e. vfrac==0) over all components, so the only work left for this function is the cosmetic cleanup — which must therefore never touch sign. Use amrex::Math::abs, the convention for device lambdas here (e.g. Source/Hydro/MOL/MOL_hydro_K.H:17), rather than std::abs.

Two calls for the maintainer: this function is new in the moving-EB WIP (#54), not a Fortran regression, so deleting the second block outright is equally defensible; and a hard 1e-12 threshold is dimensional — it will erase real data in nondimensionalized runs, so consider dropping it or restricting it to newly-uncovered cells. No other fix must land with this one.

For Source/CAMR.cpp:1291 (F024):

--- a/Source/CAMR.cpp
+++ b/Source/CAMR.cpp
@@ -1288,7 +1288,7 @@
        }
        {
             for (int n = 0; n < ncomp; ++n) {
-                if(Sarr(i, j, k, n) < 1e-12){
+                if(std::abs(Sarr(i, j, k, n)) < 1e-12){
                     Sarr(i, j, k, n) = 0.0;
                 }
             }

_{Magnitude test so only sub-1e-12 noise is zeroed for plotting; negative momenta/velocities survive. std::abs is already used in CAMR device code (e.g. Hydro_riemann.H). The bare second block still runs on covered cells after the vfrac==0 zeroing, which is idempotent, so no else is needed.}

_{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

F024 — confirmed (one verifier lens)

Lens 1 (refutation attempt): Second block at 1289-1295 is a bare '{...}' (no else, no condition) applying 'if(Sarr(i, j, k, n) < 1e-12){ Sarr(i, j, k, n) = 0.0; }' to every cell and component — signed compare zeroes all negative momenta/velocities. Called on plotMF at CAMR_io.cpp:584 inside #ifdef CAMR_USE_MOVING_EB; MovingPlane_Shock builds with USE_MOVING_EB=TRUE and plots velocities. Affects plotfiles only (plotMF is a copy), exactly as claimed.

F044 — confirmed (one verifier lens)

Reported as: ZeroingOutForPlotting's second block lacks an else and uses a signed comparison, so EVERY plotfile value below 1e-12 — including all negative values in every uncovered cell — is overwritten with zero.

Failure scenario: Any CAMR_USE_MOVING_EB build writing a plotfile (Exec/MovingEBCases/MovingPlane_Shock): negative momenta/velocities (e.g. reflected-flow ymom = -0.3) and any small-but-meaningful field values are zeroed in plotMF, so plotted moving-EB results are silently wrong wherever a field is negative.

Lens 1 (refutation attempt): Duplicate of F024. The block lacks an else (so it also re-processes covered cells just zeroed) and the signed '< 1e-12' test zeroes every negative value in every cell of plotMF. CAMR_io.cpp:584 calls ZeroingOutForPlotting(plotMF) in CAMR_USE_MOVING_EB builds; Exec/MovingEBCases/MovingPlane_Shock exists with USE_MOVING_EB=TRUE. Plotted moving-EB fields are silently wrong wherever negative.

---

_{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: F024, F044. Reviewer unit(s): CAMR-core, theme:eb-smallcell. 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