Repository navigation
Conversation
Purely additive: no call site changes, inheritance still in place. para_2d() (const + non-const) returns *this while Parallel_Orbitals derives from Parallel_2D, giving the two escape hatches a const getter cannot serve (LR_Util::setup_2d_division mutation, test-side descriptor writes). get_blacs_ctxt() completes the inherited getter family, which covers the other seven public data members.
…rate module_rt call sites
…igrate module_dftu call sites
…rate module_dm call sites
…ate pyabacus call sites
…rate module_lr call sites
…rate misc_prod call sites
…bitals, migrate module_operator_lcao call sites
…rate test_side call sites
…, migrate module_current call sites
…rate module_ao call sites
…d-inheritance # Conflicts: # source/source_esolver/esolver_ks_lcao_tddft.cpp # source/source_hamilt/module_gint/gint_dvlocal.cpp
Resolve the single conflict in output_mulliken.cpp by keeping the upstream extraction of collect_MW_nspin4 (deepmodeling#8250, which also corrects the noncollinear M_y sign) instead of the pre-extraction inline loop carried on this branch, then migrate the helper's accessor calls to the explicit para_2d() distribution now that Parallel_Orbitals no longer inherits Parallel_2D.
…cessor The noncollinear Mulliken test added by deepmodeling#8250 sets up a serial layout on a Parallel_Orbitals and sizes its ComplexMatrix from the distribution's row and column counts. `set_serial`, `nrow` and `ncol` belong to Parallel_2D, which Parallel_Orbitals no longer inherits, so route both uses through para_2d(). Without this the test target does not compile on top of current develop.
Replace the PR#TBD placeholders with the actual condition for deleting para_2d() and the implicit `operator const Parallel_2D&`, and record that the eventual accessor name carries no get_ prefix, which is the convention this repository follows. Comment text only.
ABACUS-PR-BOT
left a comment
ABACUS-PR-BOT
left a comment
There was a problem hiding this comment.
Static review of 388b4717321c: one optional-build compatibility defect and one nonblocking test suggestion. Findings are based on the full diff and affected source paths; no build or runtime tests were run.
| /// authoritative copy lives in the para_2d_ member below; re-deriving the | ||
| /// distribution from a duplicated field would silently desynchronize the | ||
| /// LCAO band/atom layers that are computed from it. | ||
| class Parallel_Orbitals |
There was a problem hiding this comment.
[P1] Finish the optional-backend migration before removing inheritance
Direct accesses to removed members remain in hsolver_lcao.cpp:383/385 (MPI+CUDA), lcao_deepks_iface.cpp:531 (DeePKS), ri_2d_comm.hpp:297/303 and BSE/RPA/EXX callers (LibRI), band_energy.cpp:343 (cuBLASMp), and the registered test_diago_pexsi.cpp:179-184 fixture. The const conversion cannot satisfy pv.member lookup, so these configurations fail to compile even when the corresponding runtime feature is unused. Please complete the migration through para_2d() across these guarded consumers and validate the affected build targets.
| po.set_atomic_trace(iat2iwt.data(), nat0, gr); | ||
| EXPECT_EQ(po.get_col_size(), gr); | ||
| EXPECT_EQ(po.get_row_size(), gr); | ||
| EXPECT_EQ(po.para_2d().get_col_size(), gr); |
There was a problem hiding this comment.
Non-blocking: the serial atom-trace branch checks only total dimensions, with its per-atom expectations commented out. Please make a focused non-MPI atom-trace test runnable in CMake and assert the atom counts and index ranges; the current module_ao test directory is entered only with ENABLE_MPI=ON. This is an existing coverage gap highlighted by the refactor.
Summary
Parallel_Orbitals's inheritance fromParallel_2Dwith composition: theblock-cyclic distribution is a single owned member exposed through
para_2d(), and theclass no longer carries any copy of the distribution state.
module_ao,misc_prod(
output_mulliken,gint_common,unk_overlap_lcao,to_w90_lcao_setup,write_proj_band_lcao,test_md_base,test_write_wfc_nao),test_side, andpyabacus.instead of relying on the inherited one.
This is one step of dissolving
Parallel_Orbitalsinto self-contained values; seedocs/adr/0001-dissolve-parallel-orbitals.mdfor the target decomposition.Notes for reviewers
operator const Parallel_2D&()is kept for now so theremaining const call sites (passing a
Parallel_Orbitalswhere aconst Parallel_2D&isexpected) keep compiling. The declaration comment states the exact condition for deleting it,
and no new call site may rely on it.
collect_MW_nspin4(which also corrects the noncollinear MullikenM_ysign) instead of thepre-extraction inline loop this branch carried, then routes the helper's accessor calls
through
para_2d(). Taking the branch side here would have silently reverted theM_yfix.touched, so no documentation update is required.
Verification
Run in a detached worktree at the merged branch head; GNU 11.4.0,
OMP_NUM_THREADS=1.abacus_basic_paralinked:[100%] Built target abacus_basic_para; identityABACUS version v3.11.0-beta12.error:lines.
100% tests passed, 0 tests failed out of 3(MODULE_AO_parallel_orbitals_test,MODULE_AO_parallel_orbitals_test_paraunder MPI,MODULE_IO_mulliken_test).--base upstream/develop --head HEAD): 6 warnings, 0 blocking.Global dependency budget is 7 added / 7 removed (net 0), i.e. migration-neutral; the added
references are pre-existing reads moved between call sites.
toolchain or build tree available here).
Two follow-ups are intentionally outside this PR: renaming
para_2d()to aget_-freeaccessor name once no caller needs the alias, and deleting
Parallel_2Ditself.