Fix verified static-analysis bugs; reject false positives - #20
Open
stanmoore1 wants to merge 2 commits into
Open
Fix verified static-analysis bugs; reject false positives#20stanmoore1 wants to merge 2 commits into
stanmoore1 wants to merge 2 commits into
Conversation
Independently audited ~111 AI-reported bugs against origin/master and the two fix branches (ai_bugfixes, copilot/fix-bugs-from-bugs-md). Applied the correct fix for each genuine defect; rejected non-bugs and regressive/ incorrect fixes. Full per-bug audit in BUG_VERIFICATION.md. Rejected as NOT-A-BUG: 40 (FFT flag convention already correct; both branches' change is a regression), 84 (speculative int widening; total size already bigint), 93 (int*sizeof already promotes to 64-bit), 102 (bounded buffer, no overflow). Corrected branch mistakes: avoided ai_bugfixes' deletion of the EPSZERO divide-by-zero guard in collide_vss.cpp / collide_vss_kokkos.cpp (bug 46); used the correct scratch-variable fix for react_qk rejection sampling (bug 72) rather than copilot's react_prob-polluting version. Completeness: ported the verified fixes for bugs 62/77/78/79/80 to the CPU copies (update.cpp, geometry.cpp) that both branches fixed only in KOKKOS; completed partial fixes (57 double memset, 58 set2, 60 dbuf, 73 ambipolar loop, 100 KOKKOS); applied 82/83 which neither branch fixed. Build: make serial links spa_serial cleanly with all non-KOKKOS fixes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKDt5aB3d3AygQPU3ihKz6
List false positives introduced by ai_bugfixes (40, 93, 102, plus cosmetic snprintf on bounded buffers), bugs where the deliverable departed from ai_bugfixes' version (46, 73/76, 75, 54), and related context on where ai_bugfixes fell short (KOKKOS-only 62/77/78/79/80; missed 3/5/9/30/47/53/ 55/56/81/82/83; adapt_grid warning regression avoided). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKDt5aB3d3AygQPU3ihKz6
stanmoore1
pushed a commit
that referenced
this pull request
Jul 25, 2026
Task #20: restore sub-timestep resolution of a depositing surface without reopening the leak that per-element advancing caused. Rebuilding the isosurface is expensive, so it is done only every Nevery steps, but the surface keeps growing in between. The collision geometry is now refreshed every step: the corner point field is advanced in time and the surface re-derived from it, so a particle is reflected off the surface where it actually is rather than where it was last rebuilt. Deriving it from the FIELD is what makes this work, and is the difference from the two approaches that failed. Advancing each element along its own normal pulls shared vertices apart and opens a gap at every joint. Displacing the vertices instead is watertight, but only describes the surface while the marching squares case is unchanged; it breaks down as soon as corner values cross thresh and vertices appear or disappear, and it leaked for any per-regeneration increment above about 0.2. Marching squares yields a valid watertight surface for whatever field it is handed, topology changes included, and neighboring cells share the corner values along their common edges, so their refreshed geometry still meets exactly. Two properties make it safe to refresh the geometry without redoing the cut-cell work: - The field is extrapolated FORWARD, so the refreshed surface always holds at least as much material as the committed one. A particle outside it is therefore outside the committed surface too, and stays consistent with the cell in/out typing and flow volumes. - The interpolation fraction is capped, globally, before any cell that holds surface would become entirely solid. Past that point the cell has no refreshed geometry to offer and the move loop would fall back to the committed surface, which by then sits behind the real one; that was exactly how the axisymmetric spherecone leaked at higher rates. The cap has to be a single number for the whole domain, since neighboring cells must evaluate shared corner values identically. The per-cell case logic is factored out of MarchingSquares::invoke() into cell_surfs(), so the rebuild and the refresh share one copy and cannot disagree. That refactor alone reproduces in.ablation.2d byte for byte. Scope: 2d and axisymmetric. 3d needs the same factoring of MarchingCubes; until then, and in Kokkos runs, whose move loop is a separate implementation, a growing surface is resolved once per Nevery as before, which is correct but coarser. MOVING is a compile-time template parameter, so ablation compiles to exactly the code it had before. Verified: zero particles lost across Nevery 1/5/20, increments up to 1.0 per regeneration, and three seeds, where the vertex approach leaked up to 20 particles; axisymmetric clean at every rate up to the guard, having previously leaked at all of them; conservation identity holds on 1 and 4 ranks; all three ablation examples byte-identical to their mpi_1 and mpi_4 reference logs; axisymmetric ablation unaffected; Kokkos serial with SPARTA_KOKKOS_EXACT builds and runs clean; tools/testing/deposition passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2aoVbCK69c9CEiap5WJnL
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.
Independently audited ~111 AI-reported bugs against origin/master and the
two fix branches (ai_bugfixes, copilot/fix-bugs-from-bugs-md). Applied the
correct fix for each genuine defect; rejected non-bugs and regressive/
incorrect fixes. Full per-bug audit in BUG_VERIFICATION.md.
Rejected as NOT-A-BUG: 40 (FFT flag convention already correct; both
branches' change is a regression), 84 (speculative int widening; total
size already bigint), 93 (int*sizeof already promotes to 64-bit), 102
(bounded buffer, no overflow).
Corrected branch mistakes: avoided ai_bugfixes' deletion of the EPSZERO
divide-by-zero guard in collide_vss.cpp / collide_vss_kokkos.cpp (bug 46);
used the correct scratch-variable fix for react_qk rejection sampling
(bug 72) rather than copilot's react_prob-polluting version.
Completeness: ported the verified fixes for bugs 62/77/78/79/80 to the CPU
copies (update.cpp, geometry.cpp) that both branches fixed only in KOKKOS;
completed partial fixes (57 double memset, 58 set2, 60 dbuf, 73 ambipolar
loop, 100 KOKKOS); applied 82/83 which neither branch fixed.
Build: make serial links spa_serial cleanly with all non-KOKKOS fixes.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01VKDt5aB3d3AygQPU3ihKz6