fix(sparse): zero-init UMFPACK solution vectors (non-deterministic Phi garbage)#176
Open
marjohma wants to merge 2 commits into
Open
fix(sparse): zero-init UMFPACK solution vectors (non-deterministic Phi garbage)#176marjohma wants to merge 2 commits into
marjohma wants to merge 2 commits into
Conversation
…d reads The SuiteSparse complex/real solve routines (sparse_solve_suitesparse*) ALLOCATE the solution vectors xx/xz (or x) but never initialize them. umf4zsol does not write solution entries for decoupled/empty matrix rows (e.g. field-free DOFs in a localised Poisson solve), so those cells were copied straight into b (=DCMPLX(xx,xz)) carrying uninitialized heap. Symptom: KIM Phi_m.dat showed non-deterministic garbage (e.g. 2.6e+243) in a few inner-boundary cells on ~1/6 of runs; re-running the same binary gave clean zeros. Pre-existing latent bug, surfaced by the fortnum migration perturbing heap/alloc layout (the solve code itself is unchanged). Zero-initializing xx/xz/x makes decoupled DOFs come out as 0 (the correct field-free value). Confirmed: 6/6 clean runs on the 33353_2900 m7 ES case, Phi_MA_ideal still matching the reference to <3e-8. Note: KIM/src/math/sparse_mod.f90 is a stale unused copy of this file (KIM links the shared QL-Balance 'sparse' target); it should be deleted.
KIM/src/math/sparse_mod.f90 was a divergent copy no longer compiled: KIM links the shared 'sparse' target built from QL-Balance/src/base/ sparse_mod.f90 (KIM/src/CMakeLists.txt), and it is absent from CMakeSources.in. Keeping it invites editing the wrong file (it nearly masked the sparse-solver fix). Remove it.
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
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.
Problem
KIM's
Phi_m.dat(andPhi_MA,Phi_m_i/e) intermittently contained uninitialized-memory garbage (e.g.2.6e+243,9.6e-315) in a few inner-boundary cells of the localised electrostatic solve — on ~1 in 6 runs. Re-running the same binary on the same inputs gave clean zeros, i.e. non-deterministic.Root cause
In the SuiteSparse solve routines of
QL-Balance/src/base/sparse_mod.f90, the solution vectors (xx/xzfor complex,xfor real) areALLOCATEd but never initialized:For a field-free localised Poisson solve, the matrix has decoupled/empty rows;
umf4zsolleaves those solution entries unwritten, sob = DCMPLX(xx,xz)copies uninitialized heap into the output ->EBdat%Phi->Phi_m.dat.Pre-existing latent bug — the solve code is byte-identical across the recent fortnum migration; that migration only perturbed heap/allocation layout, which is why the garbage began surfacing (the archived pre-fortnum output happened to get clean zeros).
Fix
Zero-initialize the solution vectors after allocation in all four SuiteSparse routines (
sparse_solve_suitesparse_b1,sparse_solve_suitesparseComplex_b1, and their_b2_loopvariants). Decoupled DOFs now come out as0— the correct field-free value.Also delete
KIM/src/math/sparse_mod.f90, a stale divergent copy that is no longer compiled (KIM links the shared QL-Balancesparsetarget; the file is absent fromCMakeSources.in). It nearly masked this fix.Verification
sparse_mod.f90.o->libsparse.a->KIM.xrelinked (build log).Phi_MA_idealmatches the archived pre-fortnum reference to <3e-8.