Conversation
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 deepmodeling#6248
…llision 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.
mohanchen
requested review from
ErjieWu and
Flying-dragon-boxing
and removed request for
Flying-dragon-boxing
September 26, 2026 10:34
added 4 commits
September 26, 2026 18:55
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'.
- 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)
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).
added 4 commits
September 26, 2026 20:40
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).
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).
…ip/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).
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).
16 tasks
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.
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.
- 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.
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.
added 9 commits
September 27, 2026 20:46
…nd 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.
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.
…arse_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.
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.
…e 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.
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.
…istency 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
…istency 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.
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.
added 3 commits
September 27, 2026 22:59
…trix 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.
…e_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.
Critsium-xy
reviewed
Sep 28, 2026
Critsium-xy
left a comment
Collaborator
There was a problem hiding this comment.
Missing tests for the cal_r_overlap_R split (PosOpBasis/PosOpCalc/PosOpWriter): no unit test covers PosOpCalc::pos_matrix or rr.csr output. The regression fixed in bce0c78 (relative instead of absolute position passed to pos_matrix) was only visible through an integration test. Per AGENTS.md rule 6 (core-module refactors), please add focused unit tests pinning the pos_matrix argument contract and r(R) output, which are also used by rt-TDDFT callers.
See inline comments for the remaining issues.
| // internal synchronization) to ensure all ranks reach Read_HContainer together. | ||
| // Read_HContainer uses fixed MPI tags 0/1; without this barrier, rank 0 can block | ||
| // in MPI_Send while slower ranks are still inside cal_exx_ions, causing deadlock. | ||
| MPI_Barrier(MPI_COMM_WORLD); |
Collaborator
There was a problem hiding this comment.
Direct MPI_Barrier calls (here and at L252, L273) violate AGENTS.md rule 9. They also only mask the root cause: Read_HContainer does p2p on MPI_COMM_WORLD with fixed tags 0/1, which can collide with LibRI/other traffic on any other call path. Suggest fixing it in Read_HContainer (duplicated communicator or unique tags) and dropping these barriers.
| this->exx_nao.exc->exx_hamilt2rho(*this->pelec, this->pv, iter); | ||
| } | ||
| } | ||
| else if (exx_info_.info_global.cal_exx) |
Collaborator
There was a problem hiding this comment.
This only fixes E_exx for init_chg=dm. In the HexxR-restart branch of OperatorEXX (init_chg=file), Eexx is never computed, so get_Eexx() returns 0 and E_exx is still printed as 0. This block also duplicates exx_hamilt2rho's set_exx logic but skips its func_type == 4 || 5 check; consider reusing that path instead.
Collaborator
There was a problem hiding this comment.
With calculation nscf, a hybrid functional, and init_chg file, OperatorEXX loads HexxR without calculating or restoring Eexx. Exx_LRI leaves that member uninitialized. The newly added unconditional NSCF set_exx(get_Eexx(), ...) therefore reads an indeterminate value.
Expected: Exchange-energy reporting and energy corrections use an initialized, valid value.
Actual: Arbitrary values, potentially NaNs, enter f_en.exx and subsequently cal_delta_eband(), corrupting reported energies. Previously, this path did not read the uninitialized member.
Fix: Restrict synchronization to paths that computed/restored Eexx; explicitly initialize and restore or calculate it for disk-loaded HexxR. Add a file-based hybrid NSCF regression test—the new test covers only init_chg dm.
…ask change Merging upstream PR deepmodeling#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.
ErjieWu
reviewed
Sep 28, 2026
ErjieWu
reviewed
Sep 28, 2026
added 13 commits
September 28, 2026 12:00
Merging upstream PR deepmodeling#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.
…o 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).
- 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).
- 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).
- 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).
…_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.
…rse 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 deepmodeling#8033. Does not touch the MPI_Barrier, Eexx-uninitialized, PARAM.*, or PosOpCalc-test items (need deeper EXX logic / governance / new tests).
…aration 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.
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
…etters - 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.
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.
- 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`.
- 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.
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
3 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.
Fix #8034 and #6248
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.
Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.Linked Issue
Fix #
Unit Tests and/or Case Tests for my changes
What's changed?
Governance Notes