Repository navigation
refactor(dfpt): move the rho mixing family to dfpt_rho_mix.cpp - #8262
Open
Zanthoxylum wants to merge 5 commits into
Open
Zanthoxylum wants to merge 5 commits into
Zanthoxylum wants to merge 5 commits into
Conversation
reset_mixing, mix_drho (plain / Kerker / Modified-Broyden) and get_residual move verbatim from dfpt_rho.cpp (646 -> 429 lines) to the new dfpt_rho_mix.cpp together with the safe_relative_defect helper, matching the Phase-1 test file test_dfpt_rho_mixing.cpp. Class declaration and call sites unchanged; build surfaces updated (Makefile.Objects, module_dfpt CMakeLists). MODULE_DFPT 10/10; si_G anchor rerun verified bit-identical frequencies and dyn file against the E5 baseline.
Zanthoxylum
added
Refactor
…dfpt_rho.cpp The MODULE_DFPT_rho_serial / phon_serial / xc_analytic_serial and pw_run_test targets compile the DFPT sources directly, so the moved reset_mixing/mix_drho/get_residual definitions must be added to their SOURCES (only the main library lists were updated). Rebuild from deleted binaries + full MODULE_DFPT ctest: 10/10.
Zanthoxylum
force-pushed
the
refactor-dfpt-rho-mix
branch
from
November 2, 2026 07:30
06d5157 to
3ecbf80
Compare
ABACUS-PR-BOT
left a comment
ABACUS-PR-BOT
left a comment
Collaborator
There was a problem hiding this comment.
No split-induced correctness regression found. Static review covered the moved method bodies, affected callers and build source lists; no build or runtime tests were run.
…om dfpt_rho.cpp The companion-test comment in dfpt_rho_mix.cpp points at the existing test_serial/dfpt_rho_serial_test.cpp, and the safe_relative_defect cross-reference in dfpt_pw_solve.cpp follows the helper to dfpt_rho_mix.cpp. After the mixing family moved, dfpt_rho.cpp no longer uses parallel_reduce.h, <functional> or <limits> -- those includes are removed. Rebuild from deleted binaries + full MODULE_DFPT ctest: 10/10.
# Conflicts: # source/source_pw/module_dfpt/test/CMakeLists.txt # source/source_pw/module_dfpt/test_serial/CMakeLists.txt
Zanthoxylum
added a commit
to Zanthoxylum/abacus-develop
that referenced
this pull request
Nov 2, 2026
Per review, test fixture classes live inside the .cpp that uses them instead of shared fixture headers: the four new headers are removed and each file carries a trimmed per-file fixture with only the members and helpers its cases need (the phon Ewald/ASR files get the minimal cell base, the core file adds the LO-TO closed-form helpers, the elec file adds the convolution references; the rho files carry a file-local RNG in an anonymous namespace -- the cross-TU inline accessor is no longer needed; the q0 core drops the unused gy_). The xc reference helpers move back into the single xc test file (550 lines: single-concern, single-source, slightly over the 500-line budget for that reason). test_dfpt_rho_mixing.cpp is renamed to test_dfpt_rho_mix.cpp to match the companion source split dfpt_rho_mix.cpp (deepmodeling#8262) exactly. The pre-existing shared test utility dfpt_serial_fixture.{h,cpp} (cell builders, real serial basis setup) is untouched. Case counts conserved (phon 20, rho 11, q0 6, xc 6); all four targets rebuilt from deleted binaries; full MODULE_DFPT ctest 10/10.
mohanchen
pushed a commit
that referenced
this pull request
Nov 2, 2026
…e boundaries (#8259) * refactor(dfpt): split the phon serial test along the source boundaries dfpt_phon_serial_test.cpp (1062 lines, 20 cases) becomes four files mirroring the sources they exercise (coding rule 11): ion_ion -> test_dfpt_phon_ewald.cpp, accumulate_electron + nlcc fill semantics -> test_dfpt_phon_elec.cpp, the apply_asr family -> test_dfpt_phon_asr.cpp (name anticipates the Phase-2 source split of the ASR family into dfpt_phon_asr.cpp), diagonalize/LO-TO/sum-rule/format -> test_dfpt_phon.cpp. The shared fixture moves to dfpt_phon_serial_fixture.h. Verbatim test moves: case count 20 = 20, MODULE_DFPT_phon_serial passes. * refactor(dfpt): split the rho serial test into response vs mixing files dfpt_rho_serial_test.cpp (880 lines, 11 cases) becomes test_dfpt_rho.cpp (compute_drho brute-force/real-space references, Gamma charge conservation, v_hartree_q closed form, the nlcc drhoc localization/phase regression) and test_dfpt_rho_mixing.cpp (the mix_drho plain/Kerker family and the Broyden convergence/reset regressions; the name anticipates the Phase-2 source split into dfpt_rho_mix.cpp). The shared RNG and fixture move to dfpt_rho_serial_fixture.h. Verbatim test moves: case count 11 = 11, MODULE_DFPT_rho_serial passes. * refactor(dfpt): split the q0 serial test along the source boundaries dfpt_q0_serial_test.cpp (668 lines, 6 cases) becomes test_dfpt_q0.cpp (build_vkb_dk finite difference, compute_eps, compute_born, the star rotation regression -> dfpt_q0.cpp) and test_dfpt_q0_pos.cpp (the pos_matrix kinetic/nonlocal velocity forms -> dfpt_q0_pos.cpp). The shared Gamma-only fixture moves to dfpt_q0_serial_fixture.h. Verbatim test moves: case count 6 = 6, MODULE_DFPT_q0_serial passes. * refactor(dfpt): extract the xc analytic serial test helpers to a fixture header dfpt_xc_analytic_serial_test.cpp (544 lines) carries ~130 lines of shared reference machinery (cubic basis builder, synthetic gaussian / cosine fields, FFT gradient / divergence references in physical units, summed libxc vrho / vsigma) used by all six cases; the helpers move to dfpt_xc_analytic_serial_fixture.h in a named namespace, bringing the test file under the 500-line budget (409 lines). Verbatim moves, case count 6 = 6, MODULE_DFPT_xc_analytic_serial passes. * docs(test): drop internal tracking labels from the split comments * fix(dfpt): C++11-safe shared RNG seed; scope dfpt_rho.h to the elec test The shared test RNG seed was an inline VARIABLE (C++17); the unit-test configuration only raises the language level to C++14. Replace it with an inline accessor returning a function-local static (one instance across the target's TUs, C++11-safe) and update the reseeding call site. dfpt_rho.h moves from the shared phon fixture header into test_dfpt_phon_elec.cpp, the only file instantiating DFPT_Rho, so the Ewald/ASR/core test files do not inherit the dependency. Both affected targets rebuilt from deleted binaries (make rc=0); rho case count 11 intact; full MODULE_DFPT ctest 10/10. * refactor(dfpt): make every serial test file self-contained (gtest style) Per review, test fixture classes live inside the .cpp that uses them instead of shared fixture headers: the four new headers are removed and each file carries a trimmed per-file fixture with only the members and helpers its cases need (the phon Ewald/ASR files get the minimal cell base, the core file adds the LO-TO closed-form helpers, the elec file adds the convolution references; the rho files carry a file-local RNG in an anonymous namespace -- the cross-TU inline accessor is no longer needed; the q0 core drops the unused gy_). The xc reference helpers move back into the single xc test file (550 lines: single-concern, single-source, slightly over the 500-line budget for that reason). test_dfpt_rho_mixing.cpp is renamed to test_dfpt_rho_mix.cpp to match the companion source split dfpt_rho_mix.cpp (#8262) exactly. The pre-existing shared test utility dfpt_serial_fixture.{h,cpp} (cell builders, real serial basis setup) is untouched. Case counts conserved (phon 20, rho 11, q0 6, xc 6); all four targets rebuilt from deleted binaries; full MODULE_DFPT ctest 10/10. * ci: retrigger checks The 'Build extra components with Intel toolchain' run hit runner disk exhaustion during the container pull (no space left on device writing libmkl_sycl_blas.so.5, retried 3x) -- an infrastructure failure on that runner, not a code change; the same check passes on the sibling branches.
mohanchen
approved these changes
Nov 2, 2026
This branch has not been deployed
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.
Summary
reset_mixing,mix_drho(the plain / Kerker / Modified-Broydenstack) and
get_residualmove verbatim fromdfpt_rho.cpp(646 → 429lines) to the new
dfpt_rho_mix.cpp, together with the file-localsafe_relative_defecthelper — the response kernels (compute_drho /drhoc / v_hartree_q) stay in
dfpt_rho.cpp, one file per concern.mix_drhostill calls theprivate
make_drho_r_(same class, cross-TU).source/Makefile.Objects,source/source_pw/module_dfpt/CMakeLists.txt(pyabacus does not list DFPT sources).
What did NOT change
No behavior change: the moved bodies are verbatim.
Verification
si_GfullDFPT rerun — phonon frequencies and the QE-format
dyn.q0outputbit-identical to the pre-move results (the run exercises plain +
Broyden mixing and the real-space manifest rebuild).
Documentation
No docs change needed: internal file organization only, no INPUT,
interface, or behavior change.