feat(librpa): add reader-v1 producer exports by AroundPeking · Pull Request #8053 · deepmodeling/abacus-develop

AroundPeking · 2026-09-30T06:04:41Z

Summary

Add versioned reader-v1 LibRPA producer outputs under OUT.librpa.
Export split wavefunction, auxiliary, and shrunk auxiliary bases.
Export LRI coefficients, full and cut Coulomb matrices, structure, band, Brillouin-zone, KS eigenvector, and velocity data.
Write distributed KS eigenvectors with MPI-IO.
Add out_librpa_reader_version and document the reader-v1 output protocol.
Add compact producer regressions for nonmagnetic and nspin = 2 cases.
Pass required runtime input and output state explicitly to the RPA producer.

…der-v1-remote-update # Conflicts: # .readthedocs.yaml # docs/conf.py

…ssion Preserve velocity_matrix.txt, rename Vxc text output and references, reject unsupported reader-v1 symmetry, abort the producer communicator on output exceptions, and validate two-rank KS assembly against same-run text data. Drop unrelated RTD changes already in develop and remove unused gauge references.

The shared MPI input fixture enables symmetry=1, so select the legacy LibRPA output version and update the corresponding parser expectation. Keep reader-v1's symmetry=-1 check and the dedicated v1 producer cases. Validated on df_iopcas_ghj with OMP_NUM_THREADS=1: - GNU 12.3 Module_IO CTest group: 53/53 passed (ENABLE_LIBRI=OFF). - Intel-built reader-v1 producer CTests: 3/3 passed, including two ranks. - Cases 60 and 64: all 46 fixture files retain their SHA256 hashes. This changes test configuration only; no user-facing INPUT behavior or generated parameter documentation changes are required.

Critsium-xy

Follow-up review. The file-split itself looks right now: rpa_lri.hpp is gone, the 16 replacements are real compiled units (69-433 lines, all under 500), rpa_lri_detail.h is declaration-only with an #ifndef guard, both CMakeLists.txt and Makefile.Objects are updated, agent_governance_check.py reports no errors and code_quality_score.py passes every new module_ri file. Remaining findings inline. Not built or tested locally; the Test job is still pending on this head.

Critsium-xy · 2026-10-08T09:54:29Z

+    this->out_bands(pelec);
+    this->out_eigen_vector(parav, psi);
+    this->out_struc(ucell);
+    this->out_bz_sampling(ucell);


out_bz_sampling runs for every out_librpa_ver, including the default 0, and the function itself has no version check. rpa_lri_bz.cpp:28 requires nk_full == nmp[0]*nmp[1]*nmp[2], but an explicit Direct/Cartesian/Line KPT list leaves nmp = {0,0,0} (only the MP branch writes it, klist.cpp:319). So a v0 run that worked before now throws and MPI_Aborts after SCF has already finished. Every existing rpa case uses Gamma plus symmetry -1, so CI does not cover this.

Please gate this call on out_librpa_ver == 1, and reject out_librpa_ver = 1 on a non-uniform grid in the INPUT check rather than aborting mid-run.

Critsium-xy · 2026-10-08T09:54:29Z

+    // the operations in the input cell's fractional row-vector convention.
+    if (this->runtime.input.symmetry == "1" && this->runtime.input.nspin != 4)
+    {
+        RpaLriDetail::write_stru_sym(ofs, ucell, *p_kv);


The k-point block above is gated on out_librpa_ver != 1, but this symmetry block is gated only on symmetry == "1", so it also runs for v0. symmetry is reset to "1" by default for a non-gamma scf (read_inp_sys.cpp:290-307), so by default a v0 stru_out.txt gains a trailing block it never had. preserves_mesh additionally throws when nmp <= 0 (rpa_lri_sym.cpp:21), which is a second v0 abort path. Please move this into the v1 branch.

Critsium-xy · 2026-10-08T09:54:29Z

+    }
+    ModuleBase::timer::end("RPA_LRI", "out_eigen_vector_v1_mpi_io");
+#endif
+#ifndef __MPI


This serial writer can never be built. ENABLE_LIBRI is a cmake_dependent_option on ENABLE_LCAO;ENABLE_MPI (CMakeLists.txt:73-74), so these translation units are only ever compiled with __MPI defined and no build even syntax-checks these ~130 lines. Please drop them.

Critsium-xy · 2026-10-08T09:54:29Z

+    const std::complex<double> zero(0.0, 0.0);
+#ifdef __MPI
+    ModuleBase::timer::start("RPA_LRI", "out_eigen_vector_v1_mpi_io");
+    try


try { opens inside this #ifdef __MPI, the guard closes at line 50, and a second #ifdef __MPI at line 51 continues the same function body and closes it with catch. It compiles, but this is the brace-across-guards pattern the previous review asked to remove. Use one guard for the whole block.

Critsium-xy · 2026-10-08T09:54:29Z

+    const int npsin_tmp = this->runtime.input.nspin == 2 ? 2 : 1;
+    const int nbands = parav.get_wfc_global_nbands();
+    const int nbasis = parav.get_wfc_global_nbasis();
+    const std::complex<double> zero(0.0, 0.0);


zero is unused. Same in rpa_lri_eigenvector_v1.cpp:31-33, where nbands, nbasis and zero are all unused in the MPI build. Leftovers from the split; they produce compiler warnings.

Critsium-xy · 2026-10-08T09:54:30Z

+        std::sort(blocks.begin(), blocks.end(), [](const V1Block& lhs, const V1Block& rhs) {
+            return lhs.pair_index < rhs.pair_index;
+        });
+        if (blocks.empty())


When a rank owns no block for this k point, no V_*_<ik>_r<rank>.dat is written at all. run_librpa_sym.py asserts the file exists for both ranks, so with more ranks or a sparser distribution this breaks. Either write a zero-block file, or document that readers must tolerate absent per-rank files.

Critsium-xy · 2026-10-08T09:54:30Z

    std::ofstream ofs;
-    const std::string out_name = (label == "") ? "out.dat" : label + "_out.dat";
-    ofs.open(global_out_dir + term + "_" + out_name,
+    const std::string out_name = term == "vxc"


This renames an existing user-visible output (vxc_out.dat -> vxc.txt) and leaves a term == "vxc" special case inside a generic writer, while kinetic, vpp_local, vpp_nonlocal and vhartree keep <term>_out.dat. The PR body does not mention the break. Please rename the whole family in a separate PR, or at minimum document the breaking change here.

Critsium-xy · 2026-10-08T09:54:30Z

        wk[i] = wk_aux[k_index];
        isk[i] = isk_aux[k_index];
    }
+    kvec_c_full.resize(kvec_c_full_aux.size() / 3);


This changes shared k-point semantics beyond LibRPA: kvec_c_full is now the global full mesh on every rank instead of a pool-local slice, and ibz_index is no longer overwritten by the identity map after reduction (klist.cpp:89). That also fixes a real out-of-bounds read in ewald_vq.hpp:121, which indexes kvec_c_full up to nkstot_nospin. Please state this in the PR body: it affects EXX and has no EXX-side test.

Critsium-xy · 2026-10-08T09:54:30Z

@@ -0,0 +1,6 @@
+etotref -34.21719023095261
+etotperatomref -17.1085951155
+CompareVXC_pass 0


catch_properties.sh:409 now skips the whole has_xc block when LIBRPA_PRODUCER_CONTRACT=1, so CompareVXC_pass and CompareOrbXC_pass never reach result.out. check_out iterates over the keys present in result.out, so these two lines are silently ignored. Remove them, or keep the comparison. Same in 64_LIBRPA_NSPIN2/result.ref.

Critsium-xy · 2026-10-08T09:54:30Z

+        COMMAND ${Python3_EXECUTABLE} ${ABACUS_TEST_DIR}/integrate/tools/run_librpa_sym.py
+                --abacus ${ABACUS_BIN_PATH} --cases ${ABACUS_TEST_DIR}/08_RI
+    )
+    set_tests_properties(08_RI_librpa_symmetry PROPERTIES PROCESSORS 2 TIMEOUT 900)


run_librpa_sym.py performs 12 two-rank runs with timeout=120 each (run_librpa_sym.py:106-107), so the worst case exceeds this 900 s budget. Both new runners also hardcode mpirun, while the rest of the suite goes through Autotest.sh and the CMake MPI discovery.

Critsium-xy

Re-review of da7d65329. Findings 1-9, 12 and 13 from the previous round are resolved. One blocking regression in the new INPUT guard is inline below; it is what the failing Test job is reporting.

Two earlier points are still open, both in the PR description:

The vxc_out.dat -> vxc.txt rename is a user-visible break and is not mentioned, and write_orb_energy still carries the term == "vxc" special case while the other terms keep <term>_out.dat.
The k-point changes are not mentioned either: kvec_c_full is now the global full mesh on every rank and ibz_index is no longer overwritten by the identity map after reduction, which also fixes an out-of-bounds read in ewald_vq.hpp:121. This affects EXX and has no EXX-side test.

Also: the summary still names the parameter out_librpa_reader_version instead of out_librpa_ver, and does not state the new hard requirement that out_librpa_ver = 1 needs a uniform Monkhorst-Pack grid.

Minor: esolver_fp.cpp drops from 58 to 56 in code_quality_score.py (already under the 60 line before this PR). Extracting the guard into a helper would keep LibRPA-specific validation out of the generic FP entry point.

Not built or run locally; based on static review plus the CI Test job log (run 37887346672).

Critsium-xy · 2026-10-09T08:02:34Z

    const double kspacing[3] = {this->inp_->kspacing[0], this->inp_->kspacing[1], this->inp_->kspacing[2]};
    const double koffset[3] = {this->inp_->koffset[0], this->inp_->koffset[1], this->inp_->koffset[2]};
    this->kv.set(ucell, ucell.symm, inp.kpoint_file, inp.nspin, ucell.G, ucell.latvec, GlobalV::ofs_running, GlobalV::ofs_warning, use_ibz, global_out_dir, gamma_only_local, kspacing, this->inp_->kmesh_type, koffset);
+    if (inp.rpa && inp.out_librpa_ver == 1)


get_is_mp() is only valid on rank 0, so this guard fires on every other rank.

is_mp is set in read_mp_mesh (klist.cpp:303,309), but read_kpoints returns immediately for my_rank != 0 (klist.cpp:218), and mpi_k() broadcasts nkstot, nkstot_nospin, nmp and koffset but not is_mp (klist.cpp:600-615). Every pre-existing get_is_mp() caller sits inside reduce_by_symmetry, which is itself rank-0 only, so the value never needed broadcasting; this is the first all-ranks consumer.

Result: rank 0 passes, every rank >= 1 hits WARNING_QUIT -> QUIT(1) -> exit(1), so any out_librpa_ver = 1 run on 2 or more ranks dies before SCF. The CI Test job confirms it: 08_RI_librpa_producer (-n 1) passes, while 08_RI_librpa_producer_mpi and 08_RI_librpa_symmetry fail with Process name: [[63785,1],1] / Exit code: 1 right after rank 0 prints DONE : INIT K-POINTS. Non-root stdout is suppressed, so the message is invisible.

Two possible fixes: drop get_is_mp() from the condition - nmp is written only on the MP path and is broadcast, so nmp[d] > 0 && prod(nmp) == nkstot_nospin is equivalent and rank-safe - or add Parallel_Common::bcast_bool(this->is_mp) in K_Vectors::mpi_k.

AroundPeking and others added 14 commits September 30, 2026 12:21

test(rpa): add LibRPA reader-v1 producer regression

095bae5

feat(librpa): add reader-v1 producer output

2949be1

feat(librpa): export reader-v1 shrink data

c8bfb47

feat(librpa): export split reader-v1 basis files

8138327

feat(librpa): export reader-v1 producer artifacts

e7ba219

feat(librpa): write reader-v1 KS eigenvectors with MPI-IO

7a3c45d

test(librpa): tighten reader-v1 producer contract

14d10bb

fix(librpa): retain RPA output runtime state

b5ab835

fix(librpa): restore velocity producer output

df54add

fix(librpa): register reader-v1 output protocol

71db95a

refactor(rpa): pass producer runtime state explicitly

3a6effd

fix(librpa): restore legacy MPI writer and upstream timer API

f68794b

docs: sync generated input parameter index

c7365b3

test(librpa): align reader-v1 producer contract

1857625

mohanchen added the Features Needed The features are indeed needed, and developers should have sophisticated knowledge label Oct 2, 2026

mohanchen reviewed Oct 2, 2026

View reviewed changes

Comment thread docs/advanced/input_files/input-main.md Outdated

Comment thread source/source_io/test/read_input_ptest.cpp Outdated

Comment thread source/source_lcao/module_ri/rpa_lri.hpp Outdated

Comment thread tests/08_RI/60_LIBRPA_RPA_READER_V1/producer_reference/aux_basis.txt

codex and others added 11 commits October 3, 2026 10:36

feat(rpa): compact reader-v1 producer outputs

0958896

fix(rpa): make split producer translation unit self-contained

0534888

fix(rpa): balance split implementation guards

2dcbfa5

Merge upstream develop into reader-v1

28a15d8

fix: use test helper for reader version checks

4df5d3c

fix(docs): use Sphinx latexpdf make mode

b7e0098

fix(docs): use standard bra-ket LaTeX

a5c49c4

fix(docs): define LaTeX math aliases

15b7123

Merge remote-tracking branch 'upstream/develop' into codex/pr-rpa-rea…

cef3eee

…der-v1-remote-update # Conflicts: # .readthedocs.yaml # docs/conf.py

refactor(rpa): name implementation fragments by purpose

1a30411

fix(input): shorten LibRPA reader parameter

8ea2669

mohanchen reviewed Oct 5, 2026

View reviewed changes

Comment thread tests/08_RI/60_LIBRPA_RPA_READER_V1/producer_reference/OUT.autotest/vxc.txt

Critsium-xy reviewed Oct 5, 2026

View reviewed changes

codex and others added 5 commits October 6, 2026 10:33

fix(ri): define destructor body for GCC instantiation

566684d

feat: preserve symmetry in reader-v1 handoff

91152cf

fix: bound full k-point MPI packing

a555db9

Merge branch 'develop' into pr-rpa-reader-v1

871b860

Critsium-xy reviewed Oct 8, 2026

View reviewed changes

codex added 6 commits October 8, 2026 19:34

fix(rpa): address follow-up reader-v1 review

0bba279

fix(test): pass MPI flag safely to reader-v1 runners

661ff00

fix(test): select a usable MPI launcher

49d53a2

fix(test): use CMake MPI launcher for reader-v1 checks

d42b75c

fix(test): propagate CMake MPI launcher flags

2cc917e

fix(test): use distributed solver for MPI reader checks

da7d653

Critsium-xy reviewed Oct 9, 2026

View reviewed changes

Conversation

AroundPeking commented Sep 30, 2026

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Critsium-xy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Critsium-xy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants