Skip to content

Unify DFT+U occupation-matrix output across PW and LCAO - #8242

Open
lanshuyue wants to merge 2 commits into
deepmodeling:developfrom
lanshuyue:codex/unify-dftu-ion-step-output
Open

lanshuyue wants to merge 2 commits into
deepmodeling:developfrom
lanshuyue:codex/unify-dftu-ion-step-output

Conversation

@lanshuyue

@lanshuyue lanshuyue commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Initialize per-ionic-step DFT+U occupation-matrix output in shared ESolver_KS::iter_init() for both PW and LCAO.
  • Remove duplicate preparation calls and file-output parameters from iter_init_dftu_pw() and finish_dftu_lcao().
  • Rename dftu_base_io_read.cpp and dftu_base_io_write.cpp to dftu_base_read.cpp and dftu_base_write.cpp, updating CMake and legacy Makefile source lists. The renamed source contents are unchanged.
  • In PW, when the occupation matrix was loaded from file, write occ_mat.txt at the first electronic step as well as the numbered snapshot. Both output calls now use the same readiness condition; fresh PW calculations still wait until the matrix is available.

Verification

  • Earlier KS initialization commit: CMake configured with IntelLLVM 2024, MPI, LCAO and tests enabled; syntax compilation passed for its five changed C++ translation units. Existing neighboring-checkout PW/LCAO I/O test binaries passed 2/2. The earlier python3 tools/03_code_analysis/agent_governance_check.py --staged check reported no errors.
  • Current commit, using this branch: cmake -S . -B /tmp/abacus-dftu-rename-build -DBUILD_TESTING=ON -DENABLE_LCAO=ON -DENABLE_MPI=ON -DENABLE_LIBXC=OFF -DENABLE_EXX=OFF -DENABLE_DEEPKS=OFF -DENABLE_CUDA=OFF -DENABLE_ROCM=OFF completed. CMake reported that the ENABLE_EXX/DEEPKS/CUDA/ROCM variables were unused.
  • cmake --build /tmp/abacus-dftu-rename-build --target MODULE_PW_dftu_base_test MODULE_DFTU_base_io -j 8: both targets built.
  • OMP_NUM_THREADS=1 /tmp/abacus-dftu-rename-build/source/source_pw/module_pwdft/unittests/MODULE_PW_dftu_base_test --gtest_brief=1: 27/27 passed.
  • OMP_NUM_THREADS=1 /tmp/abacus-dftu-rename-build/source/source_lcao/module_dftu/unittests/MODULE_DFTU_base_io --gtest_brief=1: 6/6 passed.
  • git diff --check and git diff --cached --check: passed.
  • A full PW restart calculation was not run locally. The first-step loaded-matrix esolver gate has no dedicated integration test in this focused change.

Review notes

  • No linked issue: this PR is a focused DFT+U output cleanup and PW/LCAO alignment.
  • The PARAM.globalv.global_out_dir reference moved from the PW solver call site to the shared KS solver in the earlier commit; the PR-wide global dependency count is unchanged.
  • No INPUT parameter or occupation-matrix file-format changes.
  • Core module impact: ESolver_KS prepares ion-step output for both basis paths; PW iteration finish uses the existing matrix-readiness state for both outputs.
  • Existing I/O tests cover file initialization and reader/writer behavior. No new test fixture was added for the source renames or one-line esolver readiness condition because constructing an ESolver_KS fixture would substantially expand this PR. Full PW/LCAO runtime validation remains for CI/review.
  • No governance exception requested.

@mohanchen

Copy link
Copy Markdown
Collaborator

reopen when it is not a draft

@mohanchen mohanchen closed this Nov 1, 2026
@lanshuyue lanshuyue reopened this Nov 1, 2026
@lanshuyue
lanshuyue marked this pull request as ready for review November 1, 2026 17:26
@lanshuyue lanshuyue changed the title Move DFT+U ion-step output preparation to shared KS iteration Unify DFT+U occupation-matrix output across PW and LCAO Nov 1, 2026
@lanshuyue lanshuyue added DFT+U Issues related to DFT plus U function Refactor Refactor ABACUS codes labels Nov 1, 2026

@ABACUS-PR-BOT ABACUS-PR-BOT 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.

Static review at f34f8ee found no implementation defect in the shared output preparation, readiness gate, or source renames. A regression coverage suggestion follows. No builds, runtime tests or CI diagnostics were performed; author-reported checks were not independently rerun.

const std::string& global_out_dir = PARAM.globalv.global_out_dir;
const bool latest_ready = (iter > 1 || istep > 0);
if (latest_ready)
const bool occmat_ready = (iter > 1 || istep > 0 || this->dftu_->is_occmat_ready());

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.

Non-blocking: The existing tests exercise loaded readiness and output helpers separately, so they would still pass if this gate reverted to iter > 1 || istep > 0. Add a one-step file-loaded PW restart case that checks occ_mat.txt contains the first-step matrix, with a fresh-start control and a check that numbered output does not retain stale sections.

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.

3 participants