Skip to content

Refactor DFT+U occupation reader for strict compact input - #8166

Open
lanshuyue wants to merge 8 commits into
deepmodeling:developfrom
lanshuyue:codex/dftu-base-io-refactor-v2
Open

lanshuyue wants to merge 8 commits into
deepmodeling:developfrom
lanshuyue:codex/dftu-base-io-refactor-v2

Conversation

@lanshuyue

@lanshuyue lanshuyue commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Reminder

  • I have read AGENTS.md and docs/developers_guide/agent_governance.md.
  • I have linked an issue or explained why this PR does not need one.
  • I have added adequate unit tests and/or case tests, or explained why not.
  • I have listed the exact verification commands run and their results.
  • I have described user-visible behavior changes, including INPUT parameter changes.
  • I have explained core-module impact for ESolver, HSolver, ElecState, Hamilt, Operator, Psi, or other source/ changes.
  • I have requested any needed governance exception below.

Linked Issue

No linked issue. This is a focused follow-up to closed PR #8155, addressing the maintainer request to reduce the size of dftu_base_io.cpp and resubmit the reader refactor when ready.

Unit Tests and/or Case Tests for my changes

  • Commands run:
    • cmake --build build --target MODULE_PW_dftu_base_test -j4
    • cmake --build build --target module_pwdft MODULE_DFTU_base_io MODULE_DFTU_op -j4
    • cmake --build build --target io_input MODULE_IO_input_test_para -j4
    • OMP_NUM_THREADS=1 ctest --test-dir build --output-on-failure -R '^(MODULE_PW_dftu_base_test|MODULE_DFTU_base_io|MODULE_DFTU_op|MODULE_IO_input_test_para)$'
    • git diff --check upstream/develop...HEAD
    • python3 tools/03_code_analysis/agent_governance_check.py --base upstream/develop --head HEAD --format text
    • python3 tools/03_code_analysis/code_quality_score.py --format json source/source_pw/module_pwdft/dftu_base_io.cpp source/source_pw/module_pwdft/dftu_base_io_read.cpp
  • Result summary:
    • All requested build targets passed.
    • All 4 focused CTest targets passed.
    • git diff --check passed.
    • The extracted reader scores 93/100 and has no high-cyclomatic-complexity or excessive-parameter finding. Before this PR, dftu_base_io.cpp scored 0 (raw score -52), was 1345 lines, and read_occup_m had cyclomatic complexity 40. After extraction, dftu_base_io.cpp is 996 lines and scores 6; dftu_base_read.cpp is below 500 lines and scores 93; their average is 49.5.
    • Governance check has no blocking findings. Its global-reference warnings are migration-neutral: 2 references moved to the new reader file and 2 were removed from the old file (net 0). The documentation warning is addressed by updating the init_occ_mat INPUT help metadata; generated documentation is not edited directly.
  • Checks not run, with reason:
    • Full ABACUS integration suite was not run. The change is limited to DFT+U occupation-matrix restart parsing and has focused PW, LCAO, parser-error, and INPUT-metadata unit coverage.

What's changed?

  • Extract read_occup_m and its parser helpers into dftu_base_read.cpp, following the repository's <module>_read.cpp naming pattern, with explicit PW and LCAO test linkage.
  • Accept only the current compact occupation-matrix format written by occ_mat.txt; legacy Atom= / L= / spin= input is rejected with a filename-and-line diagnostic.
  • Replace the unsafe fixed char word[20] token buffer and ignore(150, '\n') calls with line-based std::string parsing.
  • Require exact compact headers, atom order, DFT+U channels, spin/SOC labels, finite values, matrix dimensions, complete atom coverage, and no trailing non-comment content.
  • Parse complete matrix blocks into temporary storage before updating OccupationMatrix.
  • Remove the now-redundant init_occ_mat and init_chg reader arguments; file selection remains the caller's responsibility.
  • Remove the legacy dm_onsite.txt fallback and update the init_occ_mat help text to document the compact-only input contract.
  • Add coverage for long comments, long tokens, legacy-header rejection, short/long matrix rows, missing SOC blocks, compact PW SOC, and compact LCAO real spin-basis storage.

Known Limitation and Follow-up

  • This PR does not change the current init_chg = file requirement for a DFT+U occupation-matrix file. If occ_mat.txt is absent, ABACUS still terminates before read_occup_m() is called.
  • Allowing init_chg = file to continue without an occupation-matrix file requires a separate implementation that can initialize or reconstruct the DFT+U occupation matrix safely from the available calculation state.
  • The missing-file error must remain in place until that fallback initialization is implemented and tested. A follow-up change should remove the hard requirement only together with that functionality; it is intentionally out of scope for this compact-reader refactor.

Governance Notes

  • INPUT/docs changes: No INPUT parameter or default changes. The init_occ_mat help metadata now documents the compact-only file format and current filename search order. Generated YAML/Markdown documentation is intentionally not committed.
  • Core module impact: DFT+U restart I/O only. Both PW Pauli storage and LCAO real spin-basis storage remain supported; only legacy text-file compatibility is removed. No ESolver/HSolver public interface changes.
  • Global dependency rationale: GlobalV::MY_RANK and GlobalV::ofs_running were moved with read_occup_m; PR-level added=2, removed=2, net=0.
  • Exceptions requested: none.

@mohanchen
mohanchen self-requested a review October 17, 2026 13:11
@mohanchen mohanchen added DFT+U Issues related to DFT plus U function Refactor Refactor ABACUS codes labels Oct 17, 2026

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

DFT+U Issues related to DFT plus U function Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants