Conversation
… for small unit cells Co-authored-by: Cstandardlib <49788094+Cstandardlib@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix crash in SDFT simulation with high ecutwfc value
Fix buffer overflow in H_Ewald_pw::rgen for very small unit cells / high ecutwfc
Mar 13, 2026
Collaborator
|
@copilot Add a unit test for this part. |
…-overflow regression Co-authored-by: Cstandardlib <49788094+Cstandardlib@users.noreply.github.com>
Author
Added unit tests for
|
Co-authored-by: Cstandardlib <49788094+Cstandardlib@users.noreply.github.com>
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.
SDFT simulations with very small unit cells (e.g., 4-atom deuterium at 1596 g/cc) and high plane-wave cutoffs cause heap corruption due to a fixed-size buffer overflow in the Ewald real-space sum.
Root cause
r,r2,irrarrays incompute_ewald()were allocated with a hardcodedmxr=200. For small unit cells,rmax = 4/√α/lat0grows large, andrgen()can find >200 r-vectors within the sphere — writing past the array end. The old guard (nrm > mxr) also let one out-of-bounds write through silently before printing tostd::cerr.Changes
mxrsizing: Computermaxand the loop boundsnm1/nm2/nm3(same formula asrgen()uses) before allocating the work arrays. Setmxr = (2·nm1+1)(2·nm2+1)(2·nm3+1)— a tight upper bound guaranteeing no overflow for any unit cell geometry.rmax: Previously computed redundantly inside both#ifdef __MPIand#elsebranches after allocation; now computed once, before.rgen()guard: Changednrm > mxr→nrm >= mxrand replaced silentstd::cerrwithModuleBase::WARNING_QUITincludingnrm/mxrdiagnostics. This is now a true safety net that should never trigger.Reminder
Linked Issue
Fixes the "rgen, too many r-vectors" crash and associated heap corruption.
Unit Tests and/or Case Tests for my changes
Unit tests for
H_Ewald_pw::rgen()are added insource/source_hamilt/test/rgen_test.cpp:ZeroRmax: verifiesnrm=0whenrmax=0(early-return path)SimpleCubicNearestNeighbors: for a unit cubic cell withrmax=1.5, verifies exactly 18 r-vectors (6 nearest + 12 next-nearest neighbors) are returned in ascending-magnitude orderSimpleCubicNonZeroDtau: verifies non-zerodtaucorrectly shifts all computed vectors by that offsetLargeRmaxExceedsOriginalLimit: regression test usingrmax=4.0(yielding ~499 vectors, far above the oldmxr=200hard limit); setsmxrvia the same dynamic formula added in the fix and verifies all vectors are within the sphere, properly sorted, and thatnrm > 200What's changed?
mxris now computed per-call from the actualrmaxand reciprocal lattice vectors, replacing the static value of 200 that overflowed for dense/small systems. Memory allocation and safety bounds inrgen()are now consistent.Any changes of core modules? (ignore if not applicable)
H_Ewald_pw::compute_ewald()insource_hamilt/module_ewald/H_Ewald_pw.cpp— Ewald energy used by all ESolver variants. Logic is equivalent for normal cases; only the array size changes (grows as needed for extreme cases).Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.