Skip to content

feat(librpa): add reader-v1 producer exports - #8053

Open
AroundPeking wants to merge 50 commits into
deepmodeling:developfrom
AroundPeking:pr-rpa-reader-v1
Open

AroundPeking wants to merge 50 commits into
deepmodeling:developfrom
AroundPeking:pr-rpa-reader-v1

Conversation

@AroundPeking

@AroundPeking AroundPeking commented Sep 30, 2026 •

Copy link
Copy Markdown

Summary

  • Add versioned LibRPA reader-v1 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 compact producer regressions for nonmagnetic and nspin = 2 cases.
  • Pass required runtime input and output state explicitly to the RPA producer.
  • Use the input parameter out_librpa_ver (0 = legacy output, 1 = reader-v1). out_librpa_ver = 1 requires a uniform Monkhorst-Pack k-point grid and supports symmetry-reduced grids; explicit or nonuniform KPT lists are rejected before SCF.
  • Rename the existing Vxc text output from vxc_out.dat to vxc.txt (and the corresponding local/EXX files to vxc_local.txt / vxc_exx.txt). This is a user-visible filename change; other orbital-energy terms retain <term>_out.dat.
  • Preserve the full global kvec_c_full mesh on every rank and keep the reduced ibz_index mapping. This fixes the full-grid access in ewald_vq.hpp:121 and changes shared k-point semantics used by EXX; this PR has no dedicated EXX-side regression.
  • Broadcast the Monkhorst-Pack state to all MPI ranks so the reader-v1 INPUT guard is rank-safe.

@mohanchen mohanchen added the Features Needed The features are indeed needed, and developers should have sophisticated knowledge label Oct 2, 2026
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 source/source_lcao/module_ri/rpa_lri_output_v1.h Outdated
Comment thread source/source_lcao/module_ri/rpa_lri.cpp
Comment thread .readthedocs.yaml Outdated
Comment thread source/source_lcao/module_ri/rpa_lri_output.h Outdated
Comment thread source/source_lcao/module_ri/rpa_lri_output.h Outdated
Comment thread tests/08_RI/CMakeLists.txt Outdated
…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.
codex and others added 5 commits October 6, 2026 10:33
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 Critsium-xy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread source/source_lcao/module_ri/rpa_lri.cpp Outdated
Comment thread source/source_lcao/module_ri/rpa_lri_output.cpp
Comment thread source/source_lcao/module_ri/rpa_lri_eigenvector_v1.cpp Outdated
Comment thread source/source_lcao/module_ri/rpa_lri_eigenvector_v1.cpp Outdated
Comment thread source/source_lcao/module_ri/rpa_lri_eigenvector.cpp Outdated
Comment thread source/source_lcao/module_ri/rpa_lri_coulomb_io.cpp Outdated
Comment thread source/source_io/module_hs/vxc_op_tools.cpp
Comment thread source/source_cell/klist_io.cpp
Comment thread tests/08_RI/60_LIBRPA_RPA_READER_V1/result.ref Outdated
Comment thread tests/08_RI/CMakeLists.txt Outdated

@Critsium-xy Critsium-xy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment thread source/source_esolver/esolver_fp.cpp Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Autotest.sh should not be involved with any physics information/algorithms, but I see libRPA appears in the script

# echo "$has_lowf" # test out_wfc_lcao > 0
#--------------------------------------------
if ! test -z "$has_lowf" && [ $has_lowf == 1 ]; then
if ! test -z "$has_lowf" && [ "$has_lowf" == 1 ] && [ "${LIBRPA_PRODUCER_CONTRACT:-0}" != 1 ]; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this script is only related to LCAO wave functions, the "RPA" algorithm related tags should not appear in the script

Comment thread tests/08_RI/CMakeLists.txt Outdated
)
add_test(
NAME 08_RI_librpa_producer_mpi
COMMAND ${Python3_EXECUTABLE} ${ABACUS_TEST_DIR}/integrate/validation_tools/run_librpa_mpi.py

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the CMAKE here is totally different from others, for example, the phython exe appear in the CMake, I think it would be better to be removed

@mohanchen

mohanchen commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator
  1. File dependencies around RPA control statements. Please pay special attention to which files may contain RPA-related control statements, as this creates dependencies between files. Think carefully about who depends on whom — if the dependency direction is inverted, it will lead to rather troublesome code logic relationships.
  2. Comments on key formulas. If possible, please add comments to the key formulas.
  3. Lack of comments overall. Looking through the submitted code, there are essentially no comments at all. Even with AI-assisted programming, adding appropriate comments is still helpful.
  4. Runtime errors, try/catch, and MPI deadlocks. There are quite a few runtime errors and try/catch blocks in the code. In my experience, for multi-process code, runtime errors can easily cause MPI deadlocks; using WARNING_QUIT can help reduce MPI deadlock bugs.
  5. 5. CMake style. Please be careful with how the CMake files are written. It's best not to introduce Python3 code into them, as this would add a new, non-uniform special case to the program and make the code harder to maintain.

@mohanchen

Copy link
Copy Markdown
Collaborator

esolver_fp.cpp serves as the base class, meaning it should contain the knowledge required for first-principles calculations. This class should not know about any physical quantities related to basis sets, or even orbitals. "fp" stands for first-principles. esolver_ks.cpp inherits from esolver_fp.cpp and only then acquires the concept of electronic orbitals. esolver_of.cpp, which sits parallel to esolver_ks.cpp, does not need to know anything about electronic orbitals, so esolver_fp.cpp must not contain psi in principle. The source_lcao/module_ri/rpa_lri_input.h you added should not appear here; I suggest moving it to esolver_ks_lcao.cpp instead. This is again a dependency problem: if you want to add something to k-points, it should strictly belong to k-points and have nothing to do with librpa. It is a feature added to k-points, and librpa should depend on k-points, not the other way around. Otherwise you end up with inverted dependencies — for instance, the OFDFT code would end up depending on librpa, which creates severe entanglement in the codebase.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Features Needed The features are indeed needed, and developers should have sophisticated knowledge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants