Repository navigation
Fix accumulator aliasing and Hartree atom-list indexing - #15
Merged
PeizeLin merged 6 commits intoOct 24, 2026
Merged
Conversation
Apply fac_add_Ds while merging downloaded GPU contributions through the existing tensor accumulation helper. Remove the unused GPU result map and locks; the result merge runs outside the OpenMP packing regions. Add regression coverage for signed, zero and fractional coefficients, seeded outputs, repeated calls and combined contraction labels.
This was referenced Oct 23, 2026
(cherry picked from commit fe27bb1)
(cherry picked from commit 6534722)
Use the right atom list to bound the density loop and the left atom list to select mu in the CVC loop. Add scalar-reference regressions for distinct and unequal atom partitions.
Copy the input when initializing an empty result with unit coefficient. Tensor assignment shares storage, so subsequent in-place additions otherwise change the input and other accumulators. Cover independent outputs and complex accumulation coefficients.
Stardust0831
changed the title
Upstream now contains the original PeizeLin commits already imported here. Keep the independent const-input accumulator and corrected Hartree atom-list indexing. The merged tree is identical to the previously validated PR head.
Stardust0831
changed the title
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.
Initializing an accumulator from a const tensor with coefficient 1 currently shares its storage. Later in-place accumulation can then modify the input and another result. Copy the input in that case; the existing Hartree reference test and a focused input-immutability regression verify the correction.
Two indexed Hartree loops also use the wrong atom list:
uloop bylist_J.size(), matchinglist_J[u_index].mufromlist_Iin the CVC contraction, matching the original loop domain.Distinct and unequal atom-list tests reproduce the out-of-range/missing-contribution failures and check the corrected results against independent scalar references. Accumulation tests cover all 18 contractions individually and together, seeded/empty outputs, and signed, zero, fractional and complex coefficients.
This builds on PeizeLin's implementation now in
masterat6534722. The PR adds three one-line production corrections and regression tests; the original author metadata is preserved.Validation on NVIDIA V100 GPUs with GCC 13.3/OpenMP and DDLA v0.0.4/CUDA:
Complete original build and test logs. The merge with current
masterpreserves the tested file tree exactly; merge verification and joint #16 validation.Related: #16 and deepmodeling/abacus-develop#8169.