Refactor parallel communicators, Introduce module_parallel and migrate rhog_io to domain-aware interfaces - #7899
Merged
Merged
Conversation
added 12 commits
September 1, 2026 21:25
Scope: 18 files changed, +75 -134 (net -59 lines). No behavior change. Build: verified with cmake --build build (exit=0), abacus_basic_para built. Parallel common (模板化去重) - parallel_common.cpp: collapse the 6 per-type bcast_* copy-paste wrappers (int / double / complex<double>, scalar + array) into one template <typename T> bcast_world_impl backed by Parallel_Reduce's existing MPI_Type<T> traits. Keep bcast_bool/string/char as bespoke helpers and drop the redundant extra assignment in bcast_bool. - test_parallel/CMakeLists.txt (MODULE_BASE_ParaCommon): list the transitive objects the standalone test now needs - parallel_reduce, parallel_comm, parallel_global, tool_quit, global_file, global_function, memory_recorder, timer - because parallel_common.cpp references Parallel_Reduce::MPI_Type<T>::value, which depends on the six global MPI_Comm in parallel_comm.cpp, which calls Parallel_Global::divide_mpi_groups. Parallel 2D (收紧头文件依赖, rule 3) - parallel_2d.h: drop the unused #include "source_base/parallel_comm.h" (parallel_2d.h only needed <mpi.h> for MPI_Comm); this was the single largest conduit that pulled POOL_WORLD/KP_WORLD/INT_BGROUP/BP_WORLD/ GRID_WORLD/DIAG_WORLD declarations into every user of Parallel_2D / Parallel_Orbitals. - Explicitly include source_base/parallel_comm.h in the 13 consumers that were relying on transitive include to reference the global communicators: write_hs.h, hsolver_lrtd.hpp, sto_iter.cpp / sto_tool.cpp / sto_dos.cpp / sto_elecond.cpp, chgmixing.cpp, hsolver_pw_sdft.cpp, esolver_sdft_pw.cpp, diago_bpcg_test.cpp, test_hsolver_sdft.cpp. Parallel grid (重复逻辑收敛 + 现代C++清理) - parallel_grid.h/cpp: merge zpiece_to_all and zpiece_to_stogroup into one zpiece_distribute(zpiece, iz, rho, is_sdft). The only difference between the two (~130 lines each) is the choice of communicator (MPI_COMM_WORLD vs INT_BGROUP) and the root rank used in the non-pool-0 receive path (MY_RANK vs RANK_IN_BPGROUP); both are selected with two local variables so the four send/recv branches (pool0 root copy, other-rank recv, pool-root multicast, other-pool recv) share one implementation. Also rename duplicate "case 2" labels into "case 2 / case 3". - parallel_grid.cpp::z_distribution: replace raw new int[KPAR] / delete[] startp with std::vector<int> startp(KPAR) and remove five blocks of commented-out debug output. Misc dead-code / include cleanup - parallel_reduce.h: remove the dead declaration bool check_if_equal(double& v) - never defined, never referenced anywhere in the repo. - parallel_global.cpp: drop two unused includes (parallel_common.h, parallel_reduce.h) left over from earlier refactors. Governance notes: - GlobalV budget: PR total added=3 GlobalV refs, removed=19, net_delta = -16. The 3 new refs are inside the merged zpiece_distribute function (it uses the same GlobalV::MY_POOL etc. as the original two functions, they just appear on new lines in the diff). Remaining GlobalV usage in Parallel_Grid stays for Step 1 (ProcessTopology injection). - Added header includes: parallel_2d.h now includes <mpi.h> directly (it previously got MPI_Comm via parallel_comm.h); write_hs.h and hsolver_lrtd.hpp now include parallel_comm.h because the implementations reference DIAG_WORLD and POOL_WORLD respectively and were previously hiding that dependency behind the Parallel_Orbitals -> Parallel_2D -> parallel_comm transit. - No INPUT / documentation change required: all public APIs keep the same signatures and semantics (bcast, grid reduce, Parallel_2D).
…n, ParaKmeshWorld Step 1-2: establish module_parallel/ as the new home for parallel domain abstractions. - ParaWorld: base class with tag + rank + size + comm, virtual destructor for polymorphism, protected constructors, make_serial factory for ParaCollection::add() - ParaTag: 8 domain tag string constants (pw/kmesh/bsame_kdiff/bdiff_ksame/ rgrid/diag/matrix/atom) - ParaCollection: unique_ptr<ParaWorld> container with find(tag) and find_as<T>() for safe downcast; missing tag returns static empty domain - ParaKmeshWorld: first domain subclass, extracts Parallel_Kpoints logic (k-point distribution, pool mapping, cross-pool collection, gather_kvec) into a self-contained class; tests only include para_kmesh_world.h - All tests moved to module_parallel/test/ with both serial and MPI variants - Old para_world tests in test/ and test_parallel/ removed - CMakeLists.txt and Makefile.Objects updated
Extract pool-level parallel parameters (poolnproc, poolrank, npw, npw_per, npwtot) from PW_Basis into a self-contained domain class. The actual FFT-based distribution algorithm (method1/method2) stays in PW_Basis; ParaPwWorld only holds the result: how many plane waves each process gets. Tests only include para_pw_world.h, no PW_Basis or parallel_comm.h needed.
Extract DIAG_WORLD + GlobalV::DRANK/DSIZE/DCOLOR into a self-contained domain class with drank()/dsize()/dcolor() aliases. Used by PEXSI solver and LCAO HS IO modules.
Extract GRID_WORLD + GlobalV::GRANK/GSIZE + Parallel_Grid's z-distribution tables (numz/startz/whichpro) into a self-contained domain class. Owns grid dimensions and per-process z-plane allocation. Cross-pool operations (reduce_across_pools, bcast, reduce) will be added later with explicit communicator parameters, breaking the cross-domain dependency on KP_WORLD/INT_BGROUP.
Extract INT_BGROUP + BP_WORLD + GlobalV::MY_BNDGROUP/NPROC_IN_BNDGROUP/ RANK_IN_BPGROUP into a self-contained domain class with both intra and inter group communicators. Used by SDFT band parallel and BPCG diagonalization.
Extract the process-grid part of Parallel_2D (dim0/dim1/coord) into a self-contained domain class. The actual ScaLAPACK descriptor and BLACS context management stay in Parallel_2D; this class only holds the process grid dimensions and coordinates, computed automatically from the communicator size.
Add para_comm.h/.cpp providing domain-aware communication primitives (bcast, reduce, gather, min/max) that accept const ParaWorld& instead of hardcoding MPI_COMM_WORLD or POOL_WORLD. Serial mode is a no-op (except gather_int which copies locally). Invalid worlds are skipped. Tests only need para_comm.h + para_world.h, no dependency on the old parallel_common.h or parallel_reduce.h.
Add reduce_across_pools, bcast_data, and reduce_data methods to ParaRgridWorld. These replace Parallel_Grid::reduce_across_pools, Parallel_Grid::bcast, and Parallel_Grid::reduce respectively. Key difference from old code: communicators are passed explicitly as const ParaWorld& parameters, eliminating GlobalV::KPAR/MY_POOL/ RANK_IN_POOL and global POOL_WORLD/KP_WORLD/INT_BGROUP dependencies. Serial mode: reduce_across_pools is no-op, bcast_data/reduce_data copy local slabs directly. Invalid worlds are skipped safely.
Add para_setup.h/.cpp providing: - divide_mpi_groups: utility to split nproc into num_groups (serial+MPI) - split_pools: split WORLD into k-pools + band groups, returns ParaWorld objects for pw/kmesh/bsame_kdiff/bdiff_ksame domains - split_diag_world: split for DIAG_WORLD - split_grid_world: split for GRID_WORLD - setup_para_worlds: top-level function assembling all 8 domains into a ParaCollection Also adds make_mpi/make_mpi_ptr factory methods to ParaWorld for constructing domains from MPI communicators (protected ctor stays protected, factories are the public API). Replaces Parallel_Global::divide_pools, split_diag_world, split_grid_world, and divide_mpi_groups without touching old code.
The name "para_comm" was ambiguous (communicator? communication?). Rename to para_mpi_func to match the file's role: a collection of domain-aware MPI functions (bcast/reduce/gather/min/max). Files renamed: - para_comm.h/.cpp -> para_mpi_func.h/.cpp - para_comm_test.cpp -> para_mpi_func_test.cpp - para_comm_mpi_test.cpp/.sh -> para_mpi_func_mpi_test.cpp/.sh Targets renamed accordingly (MODULE_BASE_para_mpi_func[_mpi]); gtest suites renamed to ParaMpiFuncTest/ParaMpiFuncMpiTest. No behavior change, 16/16 tests pass.
Add two top-level parallel domains for multi-image calculations (e.g. NEB replicas with independent unit cells): - para_esolver_world [color = image_id]: all processes of one esolver instance; every existing solver domain is now derived from it instead of being hard-wired to MPI_COMM_WORLD. - para_images_world [color = rank_in_esolver]: cross-image communicator connecting corresponding ranks; MPI_COMM_NULL for nimage == 1 or uneven splits, following the KP_WORLD convention. split_pools/split_diag_world/split_grid_world now take a parent MPI_Comm, and setup_para_worlds takes nimage and builds the full hierarchy: WORLD -> images split -> esolver domain -> kmesh/pw/ bgroup/diag/rgrid/matrix. The hierarchy tree is documented in para_setup.h. New MPI test (mpirun -np 4) verifies single-image degradation, 2-image split ranks/sizes, derived-domain sizes, and kpar=2 inside images; multi-image tests skip under single-rank direct execution.
End-to-end pilot migration of read_rhog/write_rhog from the old POOL_WORLD + GlobalV::RANK_IN_POOL + #ifdef __MPI pattern to the new ParaWorld-based interface. Changes: - Add barrier() to para_mpi_func (required by write_rhog's 6 barriers) - Add para_bridge.h/.cpp: temporary factory make_pw_world() that wraps POOL_WORLD into a ParaWorld, hiding #ifdef __MPI from call sites. Delete this file once ParaCollection is wired into driver init. - Migrate rhog_io.cpp: all MPI_Bcast(POOL_WORLD) -> Parallel::bcast_*, all MPI_Barrier(POOL_WORLD) -> Parallel::barrier, all GlobalV::RANK_IN_POOL -> pw_world.rank(). Zero #ifdef __MPI in the file body. - Change read_rhog signature: add const ParaWorld& parameter. - Change write_rhog signature: replace ipool/irank/nrank with const ParaWorld&. Move "only pool 0 writes" check to caller. - Update call sites: charge_init.cpp (2 read_rhog calls), esolver_fp.cpp (2 write_rhog calls), read_rhog_test.cpp. - Add barrier tests to para_mpi_func_test and para_mpi_func_mpi_test. Verification: make -j30 0 errors; ctest read_rhog 4/4 pass, module_parallel 18/18 pass. Old parallel_* test failures are pre-existing (missing .sh scripts).
added 5 commits
September 3, 2026 14:50
Without this VPATH entry, make cannot locate para_*.cpp source files for the OBJS_PARALLEL targets, causing build failure: No rule to make target 'build/obj/para_world.o'
- Add explicit nspin parameter to read_rhog() signature, making it consistent with write_rhog() which already has nspin - Remove #include of parameter.h and global_variable.h from rhog_io.cpp - No longer reads PARAM.inp.nspin anywhere - Update charge_init.cpp call sites to pass the existing local nspin var - Remove dead PARAM.input.nspin assignments and #define private public hack from read_rhog_test.cpp (rhog_io no longer consumes PARAM)
- PW_Basis* rhopw (new+delete) -> value member PW_Basis rhopw - new complex*[1] + new complex[1471] (double-delete chain) -> vector<vector<complex>> rhog_data + vector<complex*> rhog - Remove TearDown() entirely (RAII handles cleanup) - Update call sites to use &rhopw and rhog.data()
- Add std::ostream* os_warning parameter to read_rhog() and write_rhog() (no default value; callers must pass explicitly) - Replace ModuleBase::WARNING/WARNING_QUIT with a local warn() helper that writes to the injected stream on rank 0 - Remove indirect GlobalV dependencies: drop TITLE, timer, WARNING, WARNING_QUIT calls; drop timer.h include - write_rhog: replace WARNING_QUIT with warn() + return false (same flow) - Update all call sites to pass &GlobalV::ofs_warning: - charge_init.cpp (2 read_rhog calls) - esolver_fp.cpp (2 write_rhog calls) - read_rhog_test.cpp (4 read_rhog calls) - read_rhog_test.cpp main(): replace GlobalV parallel vars with locals
…state Move three file pairs from source_io/module_chgpot to source_estate root: - rhog_io.h/cpp - write_elecstat_pot.h/cpp - write_init.h/cpp Also migrate read_rhog_test.cpp to source_estate/test/. Update all include paths, CMakeLists.txt (source_io and source_estate, including test configs), and Makefile.Objects (OBJS_IO -> OBJS_SRCPW). Add AGENTS.md rules 10-11: forbid #define private public access hacks in new tests, and require test_<module>.cpp naming for new unit tests.
Qianruipku
approved these changes
Sep 3, 2026
- Add 3 new tests: WriteRoundTrip, WriteFileFail, WriteRoundTripNspin2 (write_rhog was previously 0% covered by tests) - Add OsNullptrSilent test for nullptr warning stream safety - Remove GlobalV::ofs_warning usage, use local std::ofstream + fixture helpers - Extract setup_pw_basis() fixture method to avoid duplication
…ill, add input validation - Rename namespace ModuleIO -> elecstate to match source_estate convention (rhog_io was the only source_estate file still using ModuleIO) - Replace 3x ModuleBase::GlobalFunc::ZEROS with std::fill, remove global_function.h dependency, add <algorithm> - Add defensive parameter checks at the top of read_rhog and write_rhog: - pw_basis null check - rhog null check - nspin range check (1-4) - read_rhog: nx/ny/nz > 0 check - Update all call sites in esolver_fp.cpp, charge_init.cpp, test_rhog_io.cpp - Update test expected warning strings to match new namespace
The test aborted with "MPI_Comm_rank() called before MPI_INIT" because the directory-level abacus_disable_feature_definitions(__MPI) in source_estate/test stripped __MPI from every target there. With __MPI gone, the test's main() never called MPI_Init, while the linked base/planewave libraries still issue real MPI calls. Add a target-level escape hatch: a new ABACUS_KEPT_FEATURE_DEFINITIONS target property (exposed as AddTest's KEEP_FEATURE_DEFINITIONS) lets a single target keep definitions its directory disables. Use it for this test, link planewave instead of planewave_serial, and copy the support/ data at configure time (install() is skipped by plain make+ctest). Also fix test bugs hidden while __MPI was stripped: drop const from kpar (passed by reference to divide_pools), call setup_pw_basis() in the three tests that need a valid grid, and set npwtot=1000 in InconsistentGammaOnly to trigger the expected "planewaves not used" warning. Verified: make MODULE_ESTATE_test_rhog_io && ctest -R MODULE_ESTATE_test_rhog_io -> 10/10 passed.
charge_mpi_test is a real MPI test (its main() calls MPI_Init) but does not need a separate directory now that AddTest supports KEEP_FEATURE_DEFINITIONS. Move it into test/ alongside the other module tests, keep __MPI for that target, and register its mpirun -np 4 variant there. Remove the now-empty test_mpi/ subdirectory and its add_subdirectory() call. Verified: ctest -R "MODULE_ESTATE_charge_mpi_test|MODULE_ESTATE_test_rhog_io" -> 3/3 passed (test_rhog_io 10/10, charge_mpi_test, charge_mpi_test_4np 4/4).
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
This PR introduces a new
source_base/module_parallelframework that models each parallel communication domain as a typed subclass ofParaWorld, and migratesrhog_ioas the first real-world validation target.What's new:
module_parallelpara_world.h/cpprank(),size(),comm(), tagpara_collection.h/cpppara_pw/kmesh/rgrid/diag/bgroup/matrix_world.h/cpppara_setup.h/cppMPI_Comm_splita parent comm into all child domainspara_mpi_func.h/cppbcast,reduce_all,gatherv, ...) that takeconst ParaWorld&— no globalsDesign: domain state lives in subclasses; basic communication uses free functions; cross-domain ops stay as domain methods (e.g.,
ParaRgridWorld::reduce_across_pools).Worked example:
rhog_iomigrationBefore:
rhog_io.cppreadPARAM.inp.nspinandGlobalV::ofs_warning/MY_RANKimplicitly viaModuleBase::WARNING, had no test coverage forwrite_rhog, and lived insource_io/module_chgpot/.After:
PARAM.inp.nspin→ explicitconst int nspinparameterWARNING/TITLE/timer→ injectedstd::ostream* os_warning+ParaWorld::rank()via a localwarn()helperMPI_Allgathervwith raw GlobalV pool vars →para_mpi_func::gatherv(..., pw_world)ModuleBase::GlobalFunc::ZEROS→std::fill#define private publichack removed from testPW_Basis*manual new/delete → value member +std::vectorsource_estate/rhog_io.{h,cpp}(owns-electrons home), namespaceelecstateDefensive validation added at entry:
Governance
Two new rules in
AGENTS.md:#define private publicaccess hacks in tests.test_<module_name>.cpp.Intentionally not done
write_libxc_r.cpp— its GlobalV usage is about pool topology (MY_POOL,RANK_IN_POOL), a parallel-domain problem for a future pass.test_*.cpp.Diff
Verification
module_parallel: 16 unit tests (6 MPI) pass.