Repository navigation
Fix: Include index zero in nspin=4 G=0 magnetization contributions - #8146
AsTonyshment wants to merge 1 commit into
Conversation
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Static review of 8afdd56dc0919eebe51a95d6619f03ba79d00246, including independent verification: no introduced correctness defect found. git diff --check passed; the trusted governance check reported warnings only. Builds, runtime tests and CI were not verified.
Non-blocking: Please record the exact integration-test command/configuration and observed result in the PR description, and add a brief rationale that no INPUT documentation update is needed for this correction to the existing metric.
| if (ig0 > 0) | ||
| // Taoni fix 2026-10-14 | ||
| // Index zero is valid; a negative index means this rank does not own G=0. | ||
| if (ig0 >= 0) |
There was a problem hiding this comment.
Non-blocking: Please add an explicit ig_gge0 == 0 regression with a zero charge residual and nonzero G=0 magnetization, covering the convergence metric and both traditional/angle mixing metrics. The existing test_chg_mix.cpp fixture places G=0 at index 2, and test_chg_drho_inner.cpp only tests nspin=1, so those tests do not distinguish > 0 from >= 0. Check the expected nonzero contribution and include a -1 no-local-G0 case.
There was a problem hiding this comment.
Maybe you are right, but to me personally I think this is a bit of over-engineering. I think no one will change it back to ig0 > 0, especially in the age of agent-assisted programming...
What's changed?
Change three$G=0$ magnetization contribution when its local index is zero.
ig0 > 0checks toig0 >= 0so that thenspin=4convergence metric and traditional/angle mixing inner products include thePS: All integration tests passed with reference values unchanged, suggesting that the defect is inactive in the tested layouts, although it can still be triggered when the$G=0$ component occupies local index zero.