Repository navigation
Conversation
Mark synchronized positions updated only after an MD drift. Initial and restart-output synchronization preserve the existing flag, so the initial force evaluation does not advance charge-extrapolation history. Add a regression for the initial and subsequent force evaluations.
ABACUS-PR-BOT
left a comment
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
The boundary correction still needs fixes for multithreaded and hybrid-gauge crossings.
Static source review at 7b3a835bc70a5f1c7db9ac1fea0abf149cda6f18; no tests were run and CI results were unavailable.
| #pragma omp parallel for schedule(dynamic) | ||
| #endif | ||
| for(int ik = 0; ik < kv.get_nks(); ik++) | ||
| for(int ik = 0; ik < nks; ik++) |
There was a problem hiding this comment.
With multiple OpenMP threads and at least two phase-active k points, these iterations call fix_k() on the same psi_last object. Its shared cursor can change before get_pointer() is read, applying a phase to the wrong k-point coefficients now that crossing shifts reach this loop. Please use independent per-k buffer addresses or serialize the loop, and cover two active non-Gamma corrections with more than one thread.
| { | ||
| const ModuleBase::Vector3<double> tmp_rshift(rshift.x, rshift.y, rshift.z); | ||
| const double arg = -kv.kvec_d[ik] * tmp_rshift * ModuleBase::TWO_PI; | ||
| const double arg = -kvec_d[ik] * tmp_rshift * ModuleBase::TWO_PI; |
There was a problem hiding this comment.
For td_stype=2, overlap folding includes exp(i A·R), but this boundary phase uses only k. At a Gamma k point it skips correction even when overlapping atoms acquire different lattice shifts with a nontrivial A-dependent relative phase, leaving the saved overlap inconsistent with a fresh fold. Please make the boundary transformation match the saved hybrid-gauge state, using its field time, and add a crossing test with a nontrivial A-dependent phase.
| totalforceref 0.593208 | ||
| totalstressref 4.354288 | ||
| totaltimeref 1.55 | ||
| etotref -30.89742338434547 |
There was a problem hiding this comment.
Non-blocking: Please record the exact CTest/case commands, executable revision and backend/rank/thread settings used here, plus how this replacement reference and the reported develop negative control were produced. The changed geometry and time stepping make that provenance useful for reproducing the regression.
Linked Issue
Fixes #8221.
What's changed?
Restore legacy MD wrapping, displacement and boundary-shift propagation, refresh
ionic_position_updated, and correct phase updates for gathered H/S matrices. Keep k-point corrections thread-safe and include the saved hybrid-gauge vector potential. Reuse case 06 for the finite-crossing regression.Unit Tests and/or Case Tests for my changes
12 related CTests, 28 CPU RT cases and the 7 previous CI regression cases passed. Cases 06/10 also passed with 1/4 MPI ranks and 1/4 OpenMP threads. CUDA runtime was not tested locally.
Validated source:
37dea1a, CPU double precision, icpx 2022.1 / Intel MPI 2021.6 / MKL 2022.1; MKL threads = 1, build jobs ≤ 20.$fixedbelow is the absolute executable path.For the additional 06/10 runs, add
-r '^(06_NO_dir_TDDFT|10_NO_MD_boundary_shift)$'and use each combination of-n 1/4and-o 1/4.Case 06's reference (
d373187, implementation24fad25) was generated from the same INPUT/STRU/KPT using the command above with-r '^06_NO_dir_TDDFT$' -g, and reproduced with37dea1a. Unmodified develope5ad530exits normally on the identical case but fails the reference check by 17.78 eV. The fixed result agrees with a noncrossing control translated by 20/45 of the cell within 1e-9 eV.No user documentation update required: no INPUT parameters or output fields change.