Conversation
added 5 commits
September 16, 2026 09:02
…ol flow Mechanical cleanup as the first step of the module_charge governance refactor: convert leading tabs to 4-space indentation (1011 occurrences across 11 files) and add braces around all single-statement if/for/while bodies (11 sites). No functional change.
Introduce a MixingConfig POD that bundles the INPUT mixing parameters with the runtime globals (nspin, scf_thr_type, double_grid), and change set_mixing from a 12-argument interface to set_mixing(const MixingConfig&, double&, double&). Charge_Mixing now stores the config and reads nspin / scf_thr_type / double_grid from it instead of PARAM.inp / PARAM.globalv, removing the direct PARAM reads in set_mixing and init_mixing. The single production call site (esolver_ks.cpp) fills the config, and the unit test drives set_mixing via a make_cfg() helper. The '#define private public' access hack is kept for now with a TODO: the test still must write Parameter::input/sys, Charge::_space_* and XC_Functional privates, which need the Step 4/5 global-state parameterization before it can be removed. Verified: make -j30 MODULE_ESTATE_charge_mixing (build_max_para_test) passes with no errors.
…th std::vector Extract the repeated two-beta mixing functor in mix_rho_recip/mix_rho_real into a make_twobeta_mix<T> template helper (6 lambda copies removed), and convert all local raw new[]/delete[] buffers in charge_mixing_rho.cpp to zero-initialized std::vector, dropping the paired ZEROS calls.
Extend MixingConfig with gamma_only_pw/domag/domag_z so mix_resid.cpp
(get_drho, get_dkin, inner_product_recip_{rho,simple,hartree,real}) no
longer reads PARAM/GlobalV; all branches now consume this->cfg_.
inner_product_recip_rho's raw pointer-array views are switched to
std::vector. Production fills the three new fields in esolver_ks, and
the test fixture gains a sync_cfg() helper to push PARAM mutations into
cfg_ for the inner-product branch tests.
Replace the six private raw _space_rho/_space_rho_save/_space_rhog/ _space_rhog_save/_space_kin_r/_space_kin_r_save buffers with std::vector, so Charge's underlying contiguous storage self-manages and the matching delete[] calls in destroy() (which relied on reading possibly-uninitialized pointers) go away. The public rho/rhog/rho_save/ rhog_save/kin_r/kin_r_save views keep their double**/complex** shape and still alias the vector memory via .data(), so all external consumers are unaffected. Tests that drove _space_* directly are adapted to resize()/.data() and drop their manual delete[] of the buffers.
mohanchen
added
Refactor
chgmixing_ks already takes a const Input_para& inp but still read PARAM.inp.mixing_restart / PARAM.inp.scf_nmax from the global. Use the inp argument instead so the function no longer reads INPUT state through the global for these two fields. PARAM.globalv.ks_run is a runtime per-process flag (set from band-parallel topology), not an input, so it is intentionally left as-is rather than threading it through the interface.
init_rho had a cyclomatic complexity of 36 from five sequential stages (file read, atomic fallback, Thomas-Fermi tau, restart load, wfc read) interleaved through shared read_error/read_kin_error flags. Extract the four branches into private methods -- read_rho_from_file, init_rho_atomic_and_tau, load_rho_from_restart, init_rho_from_wfc -- and leave init_rho as a thin sequence of stage calls. Logic is unchanged; the error flags are threaded through as parameters. The deepest stage (read_rho_from_file) now sits at complexity 19, down from 36 for the monolith. The remaining global reads inside the stages are untouched and deferred to a later parameterization step.
…tions sum_rho, cal_rho2ne and non_linear_core_correction each used Charge members only to reach a handful of scalars (nrxx/nxyz/omega) or the reciprocal-shell table (gg_uniq/ngg); the rest of each body is pure numerics. Move the three bodies into a new charge_math namespace as free functions with those values passed explicitly, and leave the Charge members as thin forwarding wrappers so no caller outside the module changes. The kernels are now unit-testable in isolation and no longer coupled to Charge state. One behavior note: the pre-quit debug line that printed sum_rho to ofs_warning is dropped so the free function stays free of global-stream dependencies. charge_math.cpp is wired into the estate library and the charge_test target.
The CMake build already picks up charge_math.cpp; mirror that in Makefile.Objects so the legacy Makefile flow links the new charge_math kernels too. The module_charge directory is already on VPATH, so adding charge_math.o to the object list is sufficient.
…ction Remove Charge::atomic_rho entirely and replace all call sites with module_charge::atomic_rho(..., rhopw), eliminating the need for a thin wrapper on the Charge class. This decouples atomic density initialization from Charge's state and improves charge.cpp quality score from 2 to 44.
scf_out_chg_tau aborted in Parallel_Grid::reduce on assert(rhoin != nullptr) because the kin_r_save[is] handed to write_vdata_palgrid was not a valid buffer. After the _space_* storage became std::vector (ecf5084), a copied/moved Charge leaves its rho/kin_r views dangling into another object's vector buffer, and a kin_r_save never allocated (ked_flag set after allocate) stays nullptr; both surface as a null rhoin deep inside MPI gather instead of at the source. Delete Charge's copy constructor/assignment so any value copy of the vector-aliasing views fails at compile time, and check kin_r_save in ctrl_output_fp before writing tau.cube so a missing allocation reports a clear message instead of tripping the MPI assert. Verification: not run locally (per user request, user compiles).
scf_out_chg_tau (LCAO, SCAN, out_chg=1, 4 MPI ranks) aborted in Parallel_Grid::reduce on assert(rhoin != nullptr). Bisecting between 83eb5d0 (good) and ecf5084 (bad) isolated the regression to ecf5084, which moved Charge's _space_* storage from raw new[] to std::vector. Root cause: with 4 ranks the FFT grid is slab-decomposed so that the last rank owns zero real-space points (nrxx == 0, confirmed via a temporary diagnostic printing fn/is/rank/nrxx at the reduce call site). Before ecf5084, _space_rho = new double[nspin * 0] == new double[0] returned a unique non-null pointer, so rho_save[is] was non-null and the assert passed. After the change, an empty vector's .data() returns nullptr, so the rank with nrxx == 0 handed a null rhoin to reduce and tripped the assert (Debug) or fed MPI_Gatherv a null buffer (Release). A rank with nrxx == 0 is legitimate: MPI_Gatherv is invoked with sendcount 0 and ignores the send buffer. Relax the assert to only flag a null buffer when nrxx != 0, and revert the now-unneeded kin_r_save guard in ctrl_output_fp (it would have falsely aborted on the nrxx == 0 rank). Verification: Release build (build_max_para_test), ran cd tests/03_NAO_multik/scf_out_chg_tau && OMP_NUM_THREADS=1 mpirun -np 4 ../../../build_max_para_test/abacus_max_para Result: exit 0, chg.cube and tau.cube written; numerical comparison against chg.cube.ref/tau.cube.ref gives maxdiff 0 (chg) and 1e-14 (tau).
added 14 commits
September 16, 2026 22:10
…ction Move set_rho_core to charge_math::set_rho_core with rho_core, rhog_core and rhopw passed explicitly instead of reading Charge state, and call charge_math::non_linear_core_correction directly. Remove the now-unused Charge::non_linear_core_correction wrapper, use std::vector for the rhocg/vg scratch buffers, update the init_scf call site, and drop the obsolete member stubs in the elecstate unit tests.
Replace the raw new[]/delete[] displacement arrays (dis_old1, dis_old2, dis_now) with std::vector and remove the hand-written destructor. This fixes a read of uninitialized pot_order when an object is destroyed before Init_CE, a memory leak when Init_CE is called repeatedly, and a double-free risk from the implicitly generated shallow copy. The copy constructor and copy assignment are deleted so the molecular-dynamics trajectory history cannot be silently forked. The unit test now checks vector sizes instead of non-null pointers.
- Rename module_charge/charge_math.{h,cpp} to chg_tools.{h,cpp} via git mv
- Change namespace charge_math to module_charge to match charge_atomic
and chgmixing in the same directory
- Update include guard CHG_TOOLS_H and TITLE/timer labels accordingly
- Update call sites in init_scf.cpp, charge.cpp, charge_init.cpp
- Update build references in Makefile.Objects and both CMakeLists.txt
Convert the stateless class Symmetry_rho into namespace module_charge
free functions and rename files for consistency:
symm_rho.{h,cpp} -> chg_symm.{h,cpp}
symm_rho_detail.h -> chg_symm_detail.h
symm_rhog.cpp -> chg_symm_detail.cpp
- 5 public functions become module_charge::symmetrize_rho / cal_rhog_symm
(2 overloads) / cal_rhog_symm_soc (2 overloads)
- 2 cross-TU helpers (psymmg/psymmg_soc) moved to module_charge::detail
via chg_symm_detail.h
- 3 internal MPI helpers moved to anonymous namespace
- Delete dead code psymm (real-space symmetrization, never called)
- Remove empty ctor/dtor and parallel_grid.h include
- Rename begin/begin_soc to cal_rhog_symm/cal_rhog_symm_soc for clarity
- Update timer/TITLE labels from "Symmetry_rho" to "module_charge"
- Migrate all 14 call sites and 1 test stub
- Remove obsolete Makefile special rule (no more name collision)
…uct_recip_simple Move MixingConfig from charge_mixing.h into its own mixing_config.h so stateless residual kernels can include the config without dragging in Charge_Mixing. Remove inner_product_recip_simple, which had no production call sites, together with its unit test.
Relocate gint_prec_ctrl.{h,cpp} and its test into module_gint, update the
include in esolver_ks_lcao.h and rewire the CMake/Makefile object lists.
…ions Rename mix_resid.cpp to chg_drho.cpp and turn inner_product_real and inner_product_recip_hartree into module_charge free functions declared in chg_drho.h; inner_product_recip_rho, which is only shared with the unit test, moves to module_charge::detail in chg_drho_detail.h. Charge_Mixing loses the three private inner-product members and mix_rho_recip/mix_rho_real bind the free functions through lambdas. get_drho/get_dkin stay as members for this step.
Move the get_drho/get_dkin implementations into file-local cal_drho/ cal_dkin free functions with all inputs explicit; the public Charge_Mixing methods become thin forwarding wrappers so esolver call sites stay unchanged.
…nctions
Move Charge_Mixing::Kerker_screen_recip/real to module_charge namespace
as free functions in chg_precond.{h,cpp}, renaming mix_precond.cpp via
git mv. Config/grid/geometry are passed explicitly via MixingConfig,
PW_Basis*, and tpiba, eliminating the function's direct read of
PARAM.inp.nspin. Replace 8 std::bind call sites in charge_mixing_rho.cpp
with lambdas, update 2 commented-out bind sites in charge_mixing_dmr.cpp,
and rewrite 12 test call sites in charge_mixing_test.cpp to construct an
independent MixingConfig instead of poking at Charge_Mixing privates.
Drop the now-unused member function declarations from charge_mixing.h.
…rename Update the non-CMake object list to track the renamed translation unit so make-based builds do not reference the deleted mix_precond.o.
Expose cal_drho/cal_dkin as module_charge free functions in chg_drho.h and let ESolver_KS call them directly with explicit arguments; add Charge_Mixing::get_mixing_config() as a const observer for the config.
Align with the chg_<feature> naming pattern used in the same directory
(chg_drho, chg_precond, chg_symm, chg_tools). Update include guard to
CHG_ROUTINE_H, the self-include in chg_routine.cpp, the entry in
source_estate/CMakeLists.txt and source/Makefile.Objects, and the three
#include sites in esolver_ks{,_pw,_lcao}.cpp. Function names
(chgmixing_ks{,_pw,_lcao}) and TITLE/timer tags are intentionally left
unchanged to keep the diff minimal.
Rename the MixingConfig header to align with the chg_* naming convention in module_charge. Update the include guard and the four in-tree includers; no CMake change is needed since the header is not listed explicitly.
added 5 commits
September 17, 2026 21:55
…tions Rename charge_mpi.cpp to chg_parallel.cpp and add chg_parallel.h, moving the three stateless Charge member functions (reduce_diff_pools, rho_mpi, kin_r_mpi) to module_charge namespace free functions that take the Charge object explicitly. Remove their declarations from charge.h and update all call sites in elecstate_pw, stress_mgga, read_wf2rho_pw and sto_iter. Rename the unit test to test_chg_parallel.cpp and update the test target name accordingly. GlobalV/PARAM reads and the direct MPI_Allreduce in reduce_diff_pools are preserved as pre-existing technical debt (migration-neutral).
- Rename module_charge/charge_atomic.{h,cpp} to chg_atomic.{h,cpp}
- Update include guard to CHG_ATOMIC_H
- Update includes in charge_init.cpp and charge_extra.cpp
- Update source paths in CMakeLists.txt, test CMakeLists.txt
- Fix stale object names in Makefile.Objects: replace
symm_rho_charge.o/symm_rhog.o with chg_symm.o/chg_symm_detail.o
…e functions
Introduce module_charge::split_dgrid / merge_dgrid in chg_uspp.{h,cpp} as
RAII, parameter-explicit replacements for Charge_Mixing::divide_data /
combine_data / clean_data, which paired raw new[] with manual delete[]
across ~160 lines of mixing code.
- chg_uspp.{h,cpp}: stateless free functions in module_charge namespace;
outputs are caller-pre-sized std::vector, no new/delete; parameter
validation via WARNING_QUIT; TITLE/timer tags preserved
- charge_mixing_rho.cpp: rho and tau double-grid paths switched to the new
functions; raw pointer aliases kept for !double_grid so the existing
mixing call sites (nspin==1/2/4) are untouched
- CMakeLists.txt (source + test): wire chg_uspp.cpp
The legacy divide_data/combine_data/clean_data members are not yet removed;
that follows in a later step after the test is updated.
…grid Drop the legacy alias-pointer assertions (EXPECT_EQ(datas, data.data()), EXPECT_EQ(datas, nullptr) after clean_data) that coupled the test to the old new[]/delete[] ownership model. The rewritten case verifies the actual contract: - split_dgrid fills smooth and high-frequency buffers with the dense data verbatim (per-element comparison) - merge_dgrid is a left-inverse of split_dgrid (output == input) - no explicit cleanup call is required: std::vector manages storage Covers nspin == 1 and nspin == 2 paths.
With the new module_charge::split_dgrid/merge_dgrid in chg_uspp.{h,cpp}
and all call sites in charge_mixing_rho.cpp migrated, the original
Charge_Mixing::divide_data / combine_data / clean_data members are dead.
- delete charge_mixing_uspp.cpp (the raw new[]/delete[] implementation)
- drop the three member declarations from charge_mixing.h
- remove charge_mixing_uspp.cpp from source/test CMakeLists.txt
- Makefile.Objects: drop charge_mixing_uspp.o, add chg_uspp.o
- refresh one stale comment in charge_mixing_rho.cpp to reference
merge_dgrid instead of the removed combine_data
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
1 participant
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.
Refactor charge density and density matrix modules