Conversation
Pr from deepmodeling
This was referenced Oct 7, 2024
mohanchen
pushed a commit
to mohanchen/abacus-mc
that referenced
this pull request
Sep 30, 2026
- angmom_op_mat.cpp: enforce one-line-per-brace style; remove redundant `#ifdef __MPI` guards around Parallel_Common::bcast_* calls (the wrappers already guard MPI internally, per governance rule deepmodeling#9). - test_angmom_op_mat.cpp: replace `#define DOUBLETHRESHOLD 1e-12` with a `constexpr double kDoubleThreshold` in an anonymous namespace; replace range-for `auto` loops with classic indexed `int` loops to avoid `auto`.
mohanchen
added a commit
that referenced
this pull request
Oct 2, 2026
* fix: output correct E_exx in LCAO nscf hybrid functional calculations In LCAO nscf calculations with hybrid functionals (e.g. HSE), the exact exchange energy E_exx was always printed as 0. The root cause was that hamilt2density() explicitly skipped calling exx_hamilt2rho() for nscf, which is the only path that calls ElecState::set_exx() to write the computed Eexx into f_en.exx. For nscf, Hexx and Eexx are already computed in the OperatorEXX constructor when init_chg=dm reads the density matrix from file. This patch adds an explicit set_exx() call in the nscf branch of hamilt2density() to sync the pre-computed Eexx to f_en.exx, so that the energy output table shows the correct E_exx value. Fixes #6248 * fix: add MPI barriers in EXX nscf path to prevent deadlock and tag collision When running LCAO nscf hybrid functional calculations with init_chg=dm, OperatorEXX reads the density matrix from dmrs*_nao.csr files. This process involves three steps: cal_exx_ions (pure local tensor calculation), Read_HContainer (MPI_Send/MPI_Recv with fixed tags 0/1), and cal_exx_elec (may use Comm_Assemble with MPI tags). The bug occurs because cal_exx_ions has no internal MPI synchronization and uses Distribute_Equally to assign atom pairs to different ranks. This causes ranks to finish at different times. When rank 0 finishes cal_exx_ions early, it enters Read_HContainer and blocks in MPI_Send waiting for other ranks to receive. However, slower ranks are still inside cal_exx_ions and cannot reach MPI_Recv, causing a deadlock. Additionally, Read_HContainer's fixed tags 0/1 can collide with cal_exx_elec's internal Comm_Assemble tags, causing MPI_ERR_TRUNCATE or cereal::Exception when messages are mismatched. This fix adds three MPI_Barrier calls: 1. After cal_exx_ions, before Read_HContainer: ensures all ranks synchronize before entering the file-reading phase. 2. After Read_HContainer, before cal_exx_elec: prevents tag collision between Read_HContainer's tags 0/1 and cal_exx_elec's internal tags. 3. After cal_exx_elec, before init_chg_dm: prevents tag collision with the subsequent Read_HContainer call in init_chg_dm. Also adds log output to running_nscf.log for both DM read operations (init_dm_from_file and OperatorEXX) so users can see when density matrices are loaded. Verified: 10 consecutive runs of tests/08_EXX/nscf_hse_multik all pass with exit code 0, no MPI errors, correct E_exx output, and both log lines present in running_nscf.log. * fix: remove redundant slash in DM file path and improve error message The global_readin_dir and readin_dir are normalized by to_dir() to always end with '/', so concatenating another '/dmrs' produced a double slash (e.g., './/dmrs1_nao.csr'). Remove the redundant '/' in both init_dm_from_file (lcao_set.cpp) and OperatorEXX (op_exx_lcao.cpp). Also improve FileReader's error message to include the filename, so users know exactly which file failed to open instead of a bare 'Error opening file'. * add an example nscf_hse_multik * refactor(tests): rename 08_EXX to 08_RI and reorganize test cases - Rename parent directory tests/08_EXX to tests/08_RI - Rename all 24 test cases to unified format: {calc}_{features}_{gamma|multik} - Update all README files with concise descriptions (≤2 lines) - Update CASES_CPU.txt with new directory names - Update CMakeLists.txt, tests/README, and CI workflow references New naming convention: - scf_hse_gamma, scf_hse_spin2_gamma, scf_hse_spin4_multik, etc. - lr_tddft_* for LR-TDDFT tests - rpa_scf_* for RPA tests - bse_lr_multik for BSE test - nscf_hse_multik (unchanged) * refactor(module_hs): remove PARAM from write_hs_sparse and write_hs_r Pass runtime output parameters (global_out_dir, global_matrix_dir, calculation, out_app_flag, nspin, gamma_only_local, npol, nlocal) explicitly through output_TR/output_dHR/output_dSR/output_SR, save_dH_sparse, write_hsr, and write_matrix_r instead of reading PARAM inside the module. Extend the existing SparseWriteOptions with calculation/out_app_flag so the sparse-file open mode no longer depends on global INPUT state. Read PARAM once at the I/O control layer (output_mat_sparse.cpp, ctrl_scf_lcao.cpp, esolver_gets.cpp) and pass down, reducing cross-layer control per AGENTS.md rule 1. Verified: cmake build succeeds; ctest -R "write_hs_r_compat_test|single_r_io_test|write_hsk_binary|cal_plpr" passes 4/4; agent_governance_check --staged net_delta=-27 (warning only). * refactor(module_hs): remove PARAM from cal_r_overlap_r Pass cal_force and nlocal through init/init_nonlocal and the private construct_orbs_and_* helpers, and pass global_out_dir, global_matrix_dir, calculation, out_app_flag, nlocal, npol through out_rR/out_rR_other, instead of reading PARAM inside the class. The sparse-file open mode and output filename branching now use explicit arguments. Callers (output_mat_sparse.cpp, esolver_gets.cpp, velocity_op.cpp, td_info.cpp) read PARAM once at the control layer and pass it down; the rt test passes explicit literals. Reduces cross-layer control per AGENTS.md rule 1. Verified: cmake build succeeds; ctest -R "write_hs_r_compat_test|single_r_io_test|write_hsk_binary|cal_plpr" passes 4/4 and ctest -R "snap_psb|velocity|td_info|tddft" passes 7/7; agent_governance_check --staged net_delta=-27 (warning only). * refactor(module_hs): remove PARAM from write_h_terms Extend WriteHParams with the INPUT/globalv values the H-component writers need (nlocal, gamma_only_local, npol, domag, domag_z, out_app_flag, calculation, global_out_dir, global_matrix_dir) and pass them down through write_hk_common / gather_and_write / write_h_exx_impl instead of reading PARAM inside module_hs. The remaining PARAM reads live at the I/O control layer (ctrl_scf_lcao.cpp), which fills the new fields. Verification: cmake --build build -j 32 (pass); ctest -R "write_hs_r_compat_test|single_r_io_test|write_hsk_binary|cal_plpr|hcontainer" (12/13 pass; MODULE_LCAO_hcontainer_para_test fails due to missing parallel_hcontainer_tests.sh, unrelated); agent_governance_check --staged net_delta=-7 (warnings only). * refactor(module_hs): remove PARAM from write_hs/write_vxc/write_vxc_lip/write_vxc_r/cal_plpr - write_hsk/save_mat: add nlocal, ks_solver, drank parameters; all callers pass them explicitly (ctrl_scf_lcao, write_h_terms, write_vxc, write_vxc_lip, write_dh, write_hsk_binary_test). - WriteHParams gains ks_solver and drank fields, filled once in ctrl_scf_lcao. - write_Vxc (LCAO and LIP) and write_Vxc_R: add dft_plus_u/gamma_only/ global_out_dir/out_ndigits/ks_solver parameters; write_orb_energy takes global_out_dir. - AngularMomentumCalculator: constructor takes out_level and gamma_only instead of reading PARAM. - Drop now-unused module_parameter/parameter.h includes from module_hs sources (also write_hs_r.cpp, write_hs_sparse.cpp, cal_r_overlap_r.cpp). Global-dependency budget note: net_delta=+3 is migration-neutral. The 23 added references are all in the I/O control layer (ctrl_scf_lcao, ctrl_runner_lcao, write_eband_terms, esolver_ks_lcaopw, write_dh) reading PARAM once to pass down explicitly; 22 uses were removed from module_hs proper, which no longer includes parameter.h. Verification: - cmake --build build -j 32: pass - OMP_NUM_THREADS=1 ctest -R "write_hsk_binary|cal_plpr|write_hs_r_compat_test| single_r_io_test|hcontainer|write_dmk": 14/15 pass; the only failure is MODULE_LCAO_hcontainer_para_test, which fails on this machine due to the missing parallel_hcontainer_tests.sh script (pre-existing, unrelated). * test(module_hs): colocate unit tests under module_hs/unittests Move the four module_hs test files from source_io/test into a new source/source_io/module_hs/unittests directory, registered by its own CMakeLists.txt (added via add_subdirectory under BUILD_TESTING): - write_hsk_binary_test (+ mpirun -np 2 parallel test) - single_r_io_test - cal_plpr_test (kept inside if(ENABLE_LCAO)) - write_hs_r_compat_test (+ mpirun -np 2 parallel test) Details: - Test sources keep their original names via git mv; no renames. - AddTest SOURCES paths re-rooted (../module_hs/x.cpp -> ../x.cpp, ../../source_* -> ../../../source_*). - cal_plpr_test.cpp: orb fixture path gains one level since the test working directory moves one level deeper in the build tree. - No INPUT or user-facing behavior change: no documentation update needed (docs/parameters.yaml and input-main.md unaffected). Verification: - cmake -S . -B build: pass - cmake --build build -j 32: pass - OMP_NUM_THREADS=1 ctest -R "write_hsk_binary|cal_plpr|write_hs_r_compat| write_hsr_binary|single_R_test": 6/6 pass (incl. both mpirun tests) - agent_governance_check --staged: only documentation-sync warning (test-only move, no docs change required). * refactor(module_hs): remove dead code with no callers Remove three confirmed dead interfaces (repo-wide grep shows no call sites; functionality is covered by the surviving paths): - cal_r_overlap_R::out_rR_other (duplicates out_rR) - ModuleIO::write_matrix_r (covered by write_hsr) - legacy output_mat_sparse(bool...) overload (sole caller already uses the MatSparseOutputOptions interface) Net -423 lines. * refactor(module_hs): accumulate CSR column indices in memory output_single_R wrote column indices to a per-rank temporary file (temp_sparse_indices.dat) only to emit them after the values, then re-read and deleted the file on every R block. Accumulate the indices in an in-memory std::vector<int> instead and emit them with indptr, removing the per-block file round-trips and the std::remove calls. The now-unread SparseWriteOptions::temp_dir field and all assignment sites (including the two unit tests) are removed. Output bytes are unchanged. * refactor(module_hs): drop unused params/vars; scope cal_plpr globals - output_dHR/output_dSR: remove the K_Vectors& kv parameter, which the implementations never used; update the two call sites (output_mat_sparse.cpp, esolver_gets.cpp). - write_vxc.hpp: remove the unused std::ofstream ofs in set_para2d_MO. - cal_r_overlap_r.cpp: remove unused tau1/tau2/dtau in out_rR. - write_hs.h: drop the commented-out includes. - cal_plpr.cpp: move _lambda_plus/_lambda_minus and the file-scope constants into an anonymous namespace, and rename the collision-prone globals i/invsqrt2 to kImag/kInvSqrt2. * fix(module_restart): drop stale temp_dir assignment in write_Hexxs_csr The removal of SparseWriteOptions::temp_dir missed this assignment site in module_restart (outside the module_hs tree scanned earlier). Delete the assignment and the temp_dir/last_slash locals that only fed it. * refine the parameters in scf_hse_soc_symm_multik * reduce the time for lr_tddft_pbe0_gamma example * test * update CASES_CPU.txt * style(module_hs): hoist mid-file includes to the top of write_hs_r.cpp Move hcontainer_funcs.h / output_hcontainer.h / ucell_io.h from the middle of the file into the top include block. No behavior change. * update write_hs_sparse.cpp * update test list * refactor(module_hs): split cal_r_overlap_R into PosOpBasis/Calc/Writer facade - Extract orbital basis and two-center integral table construction into PosOpBasis (pos_op_basis.h/.cpp), eliminating duplicated code between init() and init_nonlocal(). - Extract position-operator matrix element evaluation into PosOpCalc (pos_op_calc.h/.cpp): pos_matrix, pos_grad_matrix, pos_beta_matrix. - Extract sparse matrix output into PosOpWriter (pos_op_writer.h/.cpp): out_lat_r now calls calc_->pos_matrix() instead of duplicating the overlap_o/x/y/z inline computation. - Keep cal_r_overlap_R as a facade with unchanged public interface so all existing callers (velocity_op, td_info, td_current_io, esolver_gets, output_mat_sparse) require zero modifications. - Add explicit includes in td_info.cpp and snap_psb_half_tddft_test.cpp that were previously satisfied transitively through cal_r_overlap_r.h. - Wire new sources into CMakeLists.txt and Makefile.Objects. Verified: full build passes; MODULE_LCAO_tddft_snap_psibeta_half_test passes. * refactor(module_hs): rename cal_r_overlap_r to pos_op_mat - Rename cal_r_overlap_r.h/.cpp to pos_op_mat.h/.cpp to reflect the actual responsibility: facade for position-operator matrix elements. - Update all #include references in esolver_gets.cpp, output_mat_sparse.cpp, td_info.h, velocity_op.h, td_pot_hybrid.h, snap_psb_half_tddft_test.cpp. - Update CMakeLists.txt (source_io, deepks test, rt test) and Makefile.Objects. - Use git mv to preserve file history. * refactor(module_hs): split write_vxc*.hpp into vxc_op_*.h/.cpp - Extract common utilities into vxc_op_tools.h/.cpp - write_vxc.hpp -> vxc_op_mat.h/.cpp (LCAO MO-space Vxc) - write_vxc_r.hpp -> vxc_op_r.h/.cpp (LCAO real-space Vxc(R)) - write_vxc_lip.hpp -> vxc_op_lip.h/.cpp (LIP MO-space Vxc) - Remove default arguments (Hexxd, Hexxc, sparse_thr) - Use std::unique_ptr for dynamic allocations - Update CMakeLists.txt and all call sites - Explicit template instantiation for fixed type combinations Build verified: cmake --build build -j16 Tests passed: ctest --test-dir build -V -R MODULE_IO (47/47) Governance check: agent_governance_check.py --staged (no findings) * fix(module_hs): replace std::make_unique with C++11 compatible unique_ptr std::make_unique is C++14 only. Use explicit new + unique_ptr constructor to maintain C++11 baseline compatibility. Affected files: - pos_op_mat.cpp - vxc_op_mat.cpp - vxc_op_lip.cpp * refactor(module_hs): split write_hs_r into hsr_writer and hs_r_legacy Separate the new HContainer-based CSR writers (write_hsr, write_hcontainer_csr{,_binary}, *_gen_fname) from the legacy LCAO_HS_Arrays-based output path (output_dHR/dSR/TR/SR) into hsr_writer.{h,cpp} and hs_r_legacy.{h,cpp} respectively. Pure mechanical move: no signature or behavior changes. All include sites and CMakeLists updated. Verified: full build passes; ctest -R "MODULE_IO_write_hs_r_compat| MODULE_IO_write_hsr_binary|MODULE_IO_single_R" 3/3 passed. * refactor(module_hs): eliminate write_hs.hpp, split into hs_dense_io and hsk_writer Move the dense square-matrix writer save_mat into hs_dense_io.{h,cpp} and the H(k)/S(k) writer write_hsk into hsk_writer.{h,cpp}. Template definitions move out of the .hpp implementation header into .cpp with explicit instantiations (double, complex<double>, float, complex<float>), removing the .hpp header per governance rule 4. ctrl_scf_lcao.cpp gains an explicit module_out/filename.h include that was previously pulled in transitively through write_hs.hpp. The hsk binary unit test now links hs_dense_io.cpp instead of relying on the header implementation, and includes parallel_orbitals/parallel_comm that were previously transitive. No behavior change. Verified: full build passes; ctest -R "MODULE_IO_write_hsk_binary| MODULE_IO_write_hs_r_compat|MODULE_IO_write_hsr_binary" 4/4 passed. * refactor(module_hs): slim save_mat internals without changing output Extract anonymous-namespace gather_row<T> to unify MPI and serial row collection, replace raw new/delete with std::vector + std::fill, and keep byte-identical text/binary output. No interface change. Verification: make -j8 (pass); OMP_NUM_THREADS=1 ctest -R "MODULE_IO_write_hsk_binary|MODULE_IO_write_hs_r_compat|MODULE_IO_write_hsr_binary" 4/4 pass. * refactor(module_hs): split write_hs_sparse into hs_sparse_io + dhs_sparse_writer Separate the generic sparse-CSR format layer (RCoordinate/SparseRBlock/ SparseRMatrix/SparseWriteOptions/save_sparse + format primitives) from the multi-component derivative writer save_dH_sparse. git-rm the monolithic write_hs_sparse.{h,cpp} and update all includers (single_r_io.h, hs_r_legacy.cpp, vxc_op_r.cpp, restart_exx_csr.hpp, exx_lri_interface.hpp, write_hs_r_compat_test.cpp) plus both CMakeLists.txt files. hs_sparse_io.h gains an explicit abfs_vector3_order.h include for Abfs::Vector3_Order (previously pulled in transitively via lcao_hs_arrays.h). No behavior change; byte-identical output. Verification: make -j8 (pass); OMP_NUM_THREADS=1 ctest -R "MODULE_IO_write_hs_r_compat|MODULE_IO_write_hsk_binary|MODULE_IO_write_hsr_binary" 4/4 pass. * refactor(module_hs): introduce MatROutputOptions for legacy HS writers Bundle the ten shared format/path/runtime parameters of output_dHR, output_dSR, and output_TR into a single MatROutputOptions struct, collapsing the repeated positional lists at both call sites (output_mat_sparse.cpp and esolver_gets.cpp). output_SR keeps its template signature. Also drop the decorative GlobalV::ofs_running banner blocks and the std::cout passthrough in output_dHR/output_TR/output_SR; the substantive "data are in file" lines and all data files are unchanged. Verification: make -j8 (pass); OMP_NUM_THREADS=1 ctest -R "MODULE_IO_write_hs_r_compat|MODULE_IO_write_hsk_binary|MODULE_IO_write_hsr_binary" 4/4 pass. * refactor(module_hs): rename write_h_terms -> hterm_writer, table-drive term writers Collapse the five non-EXX write_h_* bodies onto a shared write_h_term skeleton driven by an HTermSpec descriptor (filename prefixes, CSR label, and a per-term build callback). The term-specific part is now only how hR_tmp is filled; the build -> write H(k) -> optionally write H(R) flow is written once. write_h_exx keeps its own body (different interface). No output change. Verification: make -j8 (pass); OMP_NUM_THREADS=1 ctest -R "MODULE_IO_write_hs_r_compat|MODULE_IO_write_hsk_binary|MODULE_IO_write_hsr_binary" 4/4 pass. * Refactor: sync Makefile.Objects with module_hs write_* split Update the legacy Makefile object list to track the CMake-side split of the module_hs write_* sources: - write_hs_sparse.o -> hs_sparse_io.o + dhs_sparse_writer.o - write_hs_r.o -> hsr_writer.o + hs_r_legacy.o - write_h_terms.o -> hterm_writer.o - write_hs.hpp save_mat/write_hsk -> hs_dense_io.o + hsk_writer.o (new) No source change; keeps the non-CMake build path consistent with the renamed/split translation units. * refactor(module_hs): rename cal_plpr -> angmom_op_mat for naming consistency Align with the existing pos_op_mat / vxc_op_mat naming convention in module_hs. The new name angmom_op_mat (angular momentum operator matrix) makes the file's purpose self-evident without requiring knowledge of the legacy "plpr" abbreviation. Changes: - Rename cal_plpr.h/cpp -> angmom_op_mat.h/cpp - Rename cal_plpr_test.cpp -> test_angmom_op_mat.cpp (AGENTS.md rule 11) - Add missing header guard ANGMOM_OP_MAT_H - Update all references: ctrl_scf_lcao.cpp, CMakeLists.txt, Makefile.Objects, unittests/CMakeLists.txt Verification: - Build target io_basic: PASS - Build target MODULE_IO_angmom_op_mat_test: PASS - Unit test execution blocked by missing test orbital file (tests/PP_ORB/Si_gga_8au_100Ry_2s2p1d.orb), unrelated to rename * refactor(module_hs): rename hs_r_legacy -> hsr_legacy for naming consistency The old name hs_r_legacy visually collides with hsr_writer, making it hard to distinguish the legacy LCAO_HS_Arrays path from the new HContainer-based writers at a glance. Rename the files and update all includes, build references, and the include guard. No functional change. Verified: full make -j builds; ctest -R write_hs_r_compat passes. * fix(Makefile): add missing module_hs vxc_op objects to Makefile.Objects The Makefile-based build was missing vxc_op_mat.o, vxc_op_tools.o, vxc_op_r.o, and vxc_op_lip.o from OBJS_IO_LCAO, causing undefined reference errors during linking when ctrl_runner_lcao.cpp pulls in write_eband_terms.hpp which calls write_Vxc and orbital_energy. Also add ./source_io/module_hs to VPATH so make can locate the source files. * fix(module_hs): pass absolute position of R-translated atom to pos_matrix The PosOpBasis/Calc/Writer split (2bbadc9) moved the inline r(R) evaluation into PosOpCalc::pos_matrix, which derives the inter-center distance as R2 - R1 and expects both arguments to be absolute Cartesian positions. The rR writer still passed the already-relative vector (tau2 - tau1 + R_car) as R2, so the integration distance was shifted by -tau1: the atom-position term of the diagonal r(R) elements vanished and spurious R vectors appeared (rr.csr reported 20 R blocks instead of 9 in 03_NAO_multik/nscf_out_hsr_tr_rr). Pass the absolute position of atom ia2 translated by R_car, matching the contract used by td_current_io and td_pot_hybrid callers. Verified: nscf_out_hsr_tr_rr passes all 6 checks (HR/SR/rR/TR and energies); rr.csr matches the reference with max abs diff 4.8e-9. * refactor(module_hs): rename single_r_io to lat_r_csr and output_single_R to save_lat_r - git mv single_r_io.{h,cpp} -> lat_r_csr.{h,cpp}; test file renamed to test_lat_r_csr.cpp per test naming rule. - Rename ModuleIO::output_single_R -> ModuleIO::save_lat_r (declaration, definition, explicit template instantiations, WARNING_QUIT context strings); include guard -> LAT_R_CSR_H. - Update callers hs_sparse_io.cpp, dhs_sparse_writer.cpp, pos_op_writer.cpp; local variable single_R_options -> lat_r_options. - Rename test target MODULE_IO_single_R_test -> MODULE_IO_lat_r_csr_test (no repo-internal references to the old target name). - Wire renamed sources through Makefile.Objects and four CMakeLists.txt. Naming convention: lat_r denotes lattice vector R; csr denotes the on-disk format. Pure rename, no behavior change. Compile not yet run (user approval pending); verification to follow before next step. * test(08_RI): regenerate ref for scf_hse_soc_symm_multik after LibXC mask change Merging upstream PR #8017 applies LibXC vrho/exc down to rho=1e-10 (QE convention) instead of masking at 1e-6. The direct effect on the initial density is ~3e-10 eV, but this deliberately-minimal metallic EXX+SOC case never reaches a fixed point and stops mid-oscillation, so the shifted vxc seed moves its endpoint by ~0.11 eV; converged HSE cases shift only by ~1e-5 eV. Regenerate result.ref from the bit-reproducible np=1 run (etot -2926.9134396250 eV, stress sum 3851.624753). Symmetry and magnetic-group assertions (O_h / C_4h / nksibz=3) are unchanged. Slim the threshold file to a single energy override (1e-5 eV): measured np=4 spread is within 9e-7 eV, while force/stress/fatal fit the global defaults (1e-4 / 1e-3 / 1). Verified: np=4 three runs and np=1 one run, 8/8 checks each. * test(08_RI): regenerate ref for nscf_hse_multik after LibXC mask change Merging upstream PR #8017 applies LibXC vrho/exc down to rho=1e-10 (QE convention) instead of masking at 1e-6. This NSCF case reads a fixed density matrix and is fully deterministic: the new value is bit-identical across np=1 and np=4 runs (spread 3e-12 eV), and matches the CI cal value to all printed digits (-429.67342983 eV). Regenerate result.ref from the np=1 run (etot -429.6734298284445 eV). No threshold override is needed; the default 1e-7 eV tolerance leaves ample margin for the ~1e-11 eV MPI spread. Verified: np=1 two runs and np=4 two runs, 2/2 checks each. * refactor(module_hs): rename rr_sparse_writer to pos_op_csr and its two detail functions - git mv rr_sparse_writer.{h,cpp} -> pos_op_csr.{h,cpp}; include guard -> POS_OP_CSR_H. - Rename ModuleIO::detail::rr_sparse_has_payload -> lat_r_nonempty (predicate: any of the 3 direction nonzero counts is nonzero). - Rename ModuleIO::detail::finalize_rr_sparse_file -> assemble_csr (writes 3-field header + merges temp payload into final csr file). - Update caller pos_op_writer.cpp (include + 2 call sites) and write_hs_r_compat_test.cpp (include, 6 call sites, 4 TEST names, continuation indentation). - Wire renamed sources through Makefile.Objects and 4 CMakeLists.txt. Naming: pos_op denotes position operator; csr denotes on-disk format; lat_r in the predicate denotes lattice-R block. Pure rename, no behavior change. Compile and tests not yet run (user approval pending). * test(module_hs): add test_hsr_writer.cpp for filename-generation helpers - 13 TEST cases covering all 5 pure-string filename functions in hsr_writer.cpp: hsr_gen_fname (6), sr_gen_fname (4), dhr_gen_fname (3). - Naming rules verified: append/overwrite mode, step present/absent (including istep=-1), csr/dat extension selection, spin index offset (ispin+1). - CMake target MODULE_IO_hsr_writer_test compiles hsr_writer.cpp + ucell_io.cpp + parallel_orbitals.cpp + tmp_mocks.cpp, links parameter base device hcontainer. - All 13 tests pass: ctest -R MODULE_IO_hsr_writer_test (0.01s). * test(module_hs): add test_hs_dense_io.cpp for save_mat header format - 6 TEST cases covering ModuleIO::save_mat text and binary output: text header (step, dimension, gamma flag, rows/columns) for double and complex; binary dim header for double and complex. - Provides main() with MPI_Init + DIAG_WORLD=MPI_COMM_WORLD (same pattern as write_hsk_binary_test) since save_mat's MPI path uses MPI_Barrier and Parallel_Reduce. - Uses Parallel_2D::set_serial(dim, dim) to create a serial layout (default-constructed Parallel_2D has empty global2local_ vectors causing segfault). - CMake target MODULE_IO_hs_dense_io_test compiles hs_dense_io.cpp + parallel_2d.cpp + tool_title.cpp + base sources, links parameter. - All 6 tests pass: ctest -R MODULE_IO_hs_dense_io_test (0.63s). * refactor(module_hs): rename test files to match test_<name> convention - git mv write_hsk_binary_test.cpp -> test_hsk_writer.cpp (matches hsk_writer.cpp); CMake target MODULE_IO_write_hsk_binary_test -> MODULE_IO_hsk_writer_test (incl. parallel variant). - git mv write_hs_r_compat_test.cpp -> test_pos_op_csr.cpp (matches pos_op_csr.cpp); CMake target MODULE_IO_write_hs_r_compat_test -> MODULE_IO_pos_op_csr_test (incl. parallel variant). - Update stale file-name reference in test_hs_dense_io.cpp comment. All 4 tests pass (2 serial + 2 MPI parallel, 2.29s total). * test(module_hs): make test_hsk_writer cover write_hsk instead of save_mat test_hsk_writer.cpp included hs_dense_io.h and exercised only ModuleIO::save_mat, duplicating test_hs_dense_io.cpp, while the actual hsk_writer.cpp function ModuleIO::write_hsk had no test at all. - Move the three binary upper-triangle save_mat cases into test_hs_dense_io.cpp (HsDenseIoSaveMat suite, 6 -> 9 cases). - Rewrite test_hsk_writer.cpp around a lightweight FakeHamilt that replays per-k-point local H/S blocks: verify H(k)/S(k) files are written for every k point with the ik2iktot filename mapping and gathered upper-triangle payload, S(k) is skipped for the spin-down channel (isk == 1), and the std::complex<double> instantiation works. - CMakeLists: compile hsk_writer.cpp into the hsk test target and add np=2 ctest entries for the MPI-reduction cases of both targets. Verified: serial runs 3/3 and 9/9 passed; both np=2 ctest entries passed. * fix(module_hs): address PR review - fix dmR memory leak and dedup sparse io helpers - op_exx_lcao.cpp: replace raw `new` HContainer (never deleted, leaked per nscf EXX run) with std::unique_ptr ownership; dmR_vec is now a non-owning view passed to dm_container_to_Ds. - hs_sparse_io_detail.h: extract count_nonzeros_by_R and check_output_file_open into a shared detail header so the threshold/nonzero logic cannot diverge between the two sparse writers. - hs_sparse_io.cpp / dhs_sparse_writer.cpp: drop the verbatim-copied anonymous-namespace helpers; the unused check_output_file_open copy in hs_sparse_io.cpp (-Wunused-function) is removed. Addresses Critsium-xy review comments on PR #8033. Does not touch the MPI_Barrier, Eexx-uninitialized, PARAM.*, or PosOpCalc-test items (need deeper EXX logic / governance / new tests). * fix(test): define TestParameters at global scope to match friend declaration parameter.h declares `friend class TestParameters;`, which binds to the global-scope class. Nine test files defined the helper inside an anonymous namespace, so the friend grant did not apply and accessing PARAM.input/PARAM.sys failed with 'private within this context'. Move the helper out of the anonymous namespace and note the scope requirement in its comment. * test(module_hs): split test_pos_op_csr.cpp into per-source test files Split the 1097-line test_pos_op_csr.cpp into one-test-per-source-file per governance rule 11: - test_pos_op_csr.cpp: only lat_r_nonempty + assemble_csr (178 lines) - test_hsr_writer.cpp: merge write_hcontainer_csr/binary + write_hsr MPI gather tests into existing file; add MPI main - test_hs_sparse_io.cpp: 4 save_sparse tests (no PARAM, only GlobalV::DRANK) - test_dhs_sparse_writer.cpp: 3 save_dH_sparse tests (no PARAM) - test_output_mat_sparse.cpp: MatSparseOutputOptions default values - test_write_dmr.cpp: dmr_gen_fname + write_dmr_csr (in module_dm/test/) - csr_test_helpers.h: shared test helpers (removed sparse_format stubs that were only needed for hsr_legacy.cpp linking) - test_vxc_op_tools.cpp: new tests for vxc_op_tools functions - io_sys_var_test.cpp: replace #define private public with direct Input_para/System_para struct usage - CMakeLists.txt: register new test targets * fix(test): link device lib for module_hs tests and friend ReadInput setters - module_hs unittests: add `device` to LIBS of MODULE_IO_pos_op_csr_test and MODULE_IO_output_mat_sparse_test. memory_op.cpp explicit instantiations live in the `device` OBJECT library; parallel_device.cpp in `base` references them, so targets that link `base` must also link `device`. - read_input: add `friend class TestParameters;` so tests can drive private set_global_dir/set_globalv without `#define private public`. - io_sys_var_test: define TestParameters at global scope (matching the friend declaration) and route the 6 private calls through it. * fix(test): restore private access and use public APIs in unit tests Revert the friend-class approach introduced in a2e9fab and df78468 that broke compilation of test TUs and propagated into production TUs via read_input.h. The root cause is that `friend class TestParameters;` declared inside `namespace ModuleIO` binds to `ModuleIO::TestParameters` (not the global `::TestParameters` the tests define), so the friend grant never took effect for ReadInput's private members. - read_input.h: remove the broken `friend class TestParameters;` declaration; production TUs no longer see an undefined global TestParameters forward declaration. - io_sys_var_test.cpp, read_input_item_test.cpp: wrap read_input.h include with `#define private public`/`#undef private` to restore access to ReadInput private members (set_global_dir, set_globalv, input_lists, normalize_hs_output_options) as an interim fix. - print_info_test.cpp: use K_Vectors public test seams (set_spin_mult, read_kpoints_for_testing) instead of private members. - elecstate_pw_test.cpp: use XC_Functional::set_func_type() public static setter instead of assigning the private static member func_type directly. - read_wf2rho_pw_test.cpp: replace direct access to Charge::_space_rho with Charge::allocate() / set_rhopw() public API; remove the conflicting mock Charge::Charge()/~Charge() so the real definitions from charge.cpp link correctly. - source_io/test/CMakeLists.txt: add charge.cpp and chg_tools.cpp to MODULE_IO_read_wf2rho_pw_test SOURCES so Charge::allocate and Charge::set_rhopw symbols resolve. * refactor(hs): style cleanup for angmom_op_mat and its test - angmom_op_mat.cpp: enforce one-line-per-brace style; remove redundant `#ifdef __MPI` guards around Parallel_Common::bcast_* calls (the wrappers already guard MPI internally, per governance rule #9). - test_angmom_op_mat.cpp: replace `#define DOUBLETHRESHOLD 1e-12` with a `constexpr double kDoubleThreshold` in an anonymous namespace; replace range-for `auto` loops with classic indexed `int` loops to avoid `auto`. * refactor(hs): extract magic number and rename angmom output files - Replace 16 occurrences of the magic number 1e-12 with a named constexpr kCoeffZeroThreshold in an anonymous namespace. Comment explains why it is tighter than the project-wide sparse output threshold 1e-10: this guards an analytical zero (sqrt(l(l+1)-m(m+1)) at extremal m), not a numerical matrix element. - Rename Lx/Ly/Lz output files from `${prefix}_Lx/y/z.dat` to `lx/ly/lz_nao.txt`. The prefix parameter is kept in the signature to avoid an interface change but no longer participates in the filename. * feat(hs): add istep support to AngularMomentumCalculator for out_freq_ion - Add `istep` parameter (default -1) to `calculate()`. When istep >= 0, filenames include `g{istep+1}` (e.g., lxg1_nao.txt); when istep = -1, filenames are lx_nao.txt (overwrite mode). - Update prompt message to reflect new filename pattern. - Pass `istep_in` from ctrl_scf_lcao.cpp, which is computed by the existing out_freq_ion/out_freq_td logic. This makes the Lx/Ly/Lz matrix output follow the same frequency control as other outputs (e.g., out_hsr, out_dmr). * feat(hs): add ModuleBase::timer to out_rR and calculate - Add ModuleBase::TITLE/timer::start/timer::end to cal_r_overlap_R::out_rR and AngularMomentumCalculator::calculate, following the ABACUS governance rule #12 (timer at function entry/exit with function name as label). - Add missing #include "source_base/timer.h" in pos_op_mat.cpp. * refactor(hs): rename AngularMomentumCalculator → Angmom_op, cal_r_overlap_R → Position_op - Rename AngularMomentumCalculator class to Angmom_op in angmom_op_mat.h/cpp. - Rename cal_r_overlap_R class to Position_op in pos_op_mat.h/cpp. - Update all references in 14 files including ctrl_scf_lcao.cpp, output_mat_sparse.cpp, esolver_gets.cpp, velocity_op, td_current_io, and snap_psb_half_tddft_test.cpp. - The shorter names follow the ABACUS naming convention for operator classes (e.g., Velocity_op). * fix: post-merge compile errors (td_info includes, v_xc gga_grad, TestParameters) - td_info.h/cpp: restore includes lost in merge (abfs_vector3_order.h, parameter.h) - hterm_writer: pass new upstream gga_grad arg to XC_Functional::v_xc via WriteHParams - read_input_item_test: use TestParameters wrapper for private Parameter::input access * Unify ion-step suffix for sparse matrix outputs; fix double free in read_wf2rho test Rename outputs to carry a 1-indexed ionic-step suffix (aligned with lxg): - rr_nao.txt -> rrg{step+1}_nao.txt when istep >= 0 - trs1_nao.csr -> tr_nao.csr (drop spin index; T(R) is spin-independent), trg{step+1}_nao.csr when istep >= 0; fix g suffix placed after .csr in the md_no_append branch - dhrx/dsrx -> append g{step+1} for all steps (was only md_no_append, 0-indexed) istep == -1 keeps the no-suffix names. read_wf2rho_pw_test: remove delete[] on chg.rho/chg_ref.rho. Charge::allocate backs rho with std::vector storage (rho = _ptrs_rho.data()), so delete[] on it is a double free / heap corruption that aborts under MPI (malloc_consolidate unaligned fastbin). The Charge destructor releases the vector storage. * update example * Fix tr filename: place ion-step suffix after the tr prefix Previously the suffix was appended after the full filename, producing tr_nao.csrg1. Now strip the _nao.csr tail and re-insert it after g{step+1}, giving trg{step+1}_nao.csr, aligned with rrg/lxg naming. * Disable out_mat_dh on MPI ranks > 1 to avoid deadlock Veff::cal_dH's Hellmann-Feynman (vl) path issues a collective (Forces::cal_force_loc -> PW real2recip Alltoallv) per orbital pair inside a loop whose trip count is rank-dependent. With >1 rank the collectives are entered a different number of times per rank and the run hangs. Skip write_dH_components when GlobalV::NPROC > 1 and warn, until the HF path is made rank-lockstep. Reproducible with examples/17_relax/04_relax_with_output_lcao (out_mat_dh 1) at -np 4. * Fix MPI tag collisions in HContainer transfer; drop masking barriers Read_HContainer::read() and the serial<->parallel transfer used fixed tags 0/1/2 on MPI_COMM_WORLD, which collide with unrelated p2p traffic on other call paths and cause MPI_ERR_TRUNCATE races or deadlocks. Give them dedicated high tags (kHctTagSize/Index/Value in transfer.h) shared by both send and recv sides, and remove the three MPI_Barrier calls in op_exx_lcao.cpp that only masked this root cause. Also fixes an asymmetric tag pair: HTransPara::receive_data used tag 0 while the matching send_data used tag 2; both now use kHctTagValue. * Initialize Exx_LRI::Eexx to avoid uninitialized read in HexxR restart In the HexxR-restart branch (init_chg=file) OperatorEXX never calls cal_exx_elec, so Exx_LRI::Eexx was left uninitialized, yet the NSCF path in esolver_ks_lcao.cpp still reads get_Eexx() for the energy report. The indeterminate value could propagate into f_en.exx and cal_delta_eband(). Default-initialize Eexx to 0.0 so the report is deterministic; the value is still not the true exchange energy on that path (HexxR restart is slated for removal, so no file-format change is made). * Fix hs_sparse_io unit test link errors with hcontainer and Atom mocks MODULE_IO_hs_sparse_io_test and MODULE_IO_dhs_sparse_writer_test both include csr_test_helpers.h, which uses hamilt::AtomPair/HContainer and Atom construction. Link hcontainer OBJECT library and reuse module_hcontainer/test/tmp_mocks.cpp for the lightweight Atom stub, matching the existing MODULE_IO_hsr_writer_test pattern. * Fix dhs_sparse_writer test to read step-suffixed CSR filenames ModuleIO::save_dH_sparse writes files with a "g<step+1>" suffix when istep >= 0 (e.g. dhrxs1g6_nao.csr for istep=5), but the three tests read the suffix-less names and therefore saw empty output. Update the expected filenames and extend remove_derivative_files with an optional step parameter to clean the suffixed variants. * Rename T(R) sparse output references from trs1_nao.csr to tr_nao.csr ModuleIO::save_dH_sparse now writes T(R) files as tr_nao.csr (no spin suffix, matching the tr prefix convention), but the integration test script and documentation still referenced the old trs1_nao.csr name. Update catch_properties.sh comparison, out_mat_t description in docs, and rename the two reference files accordingly. * Clamp dH/dS sparse writer step to non-negative and rename rr ref file save_dH_sparse now uses std::max(istep, 0) for the STEP header value, consistent with hs_sparse_io and hsr_writer. This prevents STEP: -1 when out_freq_ion == 0 (istep_in defaults to -1). Also rename nscf_out_hsr_tr_rr/rr.csr.ref to rr_nao.txt.ref and update catch_properties.sh to compare against OUT.autotest/rr_nao.txt, since pos_op_writer actually outputs rr_nao.txt. * Fix cast-away-const error in dhs_sparse_writer binary writes std::ostream::write takes const char*, so reinterpret_cast to const char* directly instead of casting away qualifiers. * Revert "Disable out_mat_dh on MPI ranks > 1 to avoid deadlock" This reverts the runtime guard added in aebfad6. The guard skipped ModuleIO::write_dH_components whenever GlobalV::NPROC > 1, which broke the tests/02_NAO_Gamma/scf_out_dh integration case: its INPUT enables out_mat_dh plus the five dH component outputs, and the case runs under 4 MPI ranks, so the dhk/dtk/dvhk/dvlk/dvnlk/dvxckz files were never written and the Compare_*_pass entries all flipped to fail. The underlying multi-rank deadlock in hamilt::Veff::cal_dH's vl path is real (see reverted commit for a reproducer), but disabling the feature silently is not the right trade-off for this PR. Keep the original behaviour and leave a FIXME documenting the bug so a follow-up can fix the Hellmann-Feynman collective pattern properly. * Update rr_nao.txt.ref for unified ion-step suffix in CSR header Commit 8b2103d unified the ion-step suffix for sparse matrix outputs. The SCF path passes istep=-1 to PosOpWriter::out_lat_r, so the CSR header now writes "STEP: -1" instead of "STEP: 0". Update the two rr_nao.txt.ref files to match. Also rename scf_out_hsr_spin4/rr.csr.ref -> rr_nao.txt.ref to align with the new output filename. * fix: explicit istep >= 0 comparisons and get_s step handling - hs_sparse_io.cpp, pos_op_writer.cpp: change implicit int-to-bool conversion of istep/step to explicit `>= 0` comparison for clarity. - esolver_gets.cpp: pass -1 instead of istep to out_rR since get_s has no ionic step concept, avoiding spurious g1 suffix in filenames. - docs: update out_mat_l filenames from OUT.{suffix}_Lx.dat to lx_nao.txt / lxg{step+1}_nao.txt to match code changes. * fix: use raw istep for dH/dS filenames, clamp only header STEP value The std::max(istep, 0) clamp in save_dH_sparse made the no-suffix branch dead: SCF (istep=-1) was writing dhrxs1g1_nao.csr instead of dhrxs1_nao.csr. Use istep for filename/append decisions and clamp only the header STEP value, consistent with hs_sparse_io and hsr_writer. Also clamp the STEP value passed to assemble_csr in pos_op_writer so rr_nao.txt writes STEP: 0 instead of STEP: -1, matching the ref files. * fix: update out_mat_l description in read_inp_out.cpp to match new filenames docs/parameters.yaml is auto-generated from the C++ source; the description string in read_inp_out.cpp must be updated to keep them in sync. --------- Co-authored-by: abacus_fixer <mohanchen@pku.eud.cn>
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
2 participants
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.
No description provided.