Skip to content

DFT+U Refactor#7538

Open
mohanchen wants to merge 3 commits into
deepmodeling:developfrom
mohanchen:2026-06-30
Open

DFT+U Refactor#7538
mohanchen wants to merge 3 commits into
deepmodeling:developfrom
mohanchen:2026-06-30

Conversation

@mohanchen

Copy link
Copy Markdown
Collaborator

DFT+U Refactor

@mohanchen mohanchen requested a review from lanshuyue June 30, 2026 09:36
@mohanchen mohanchen added DFT+U Issues related to DFT plus U function Refactor Refactor ABACUS codes labels Jun 30, 2026

@lanshuyue lanshuyue 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.

I think the direction of passing explicit parameters instead of letting lower-level DFT+U I/O code read from global PARAM is good. However, I see several issues that should be addressed before merging.

The main blocking issue is the onsite.dm format compatibility. This PR changes the format written by write_occup_m() to labels like Atom=, ORBITAL=, and spin=, but read_occup_m() still expects the
old tokens atoms, L, zeta, and spin. This means a newly written onsite.dm may not be readable by the current reader. Also, the writer now prints some indices as 1-based (iat+1, is+1), while the
reader uses parsed values as array indices, which could cause off-by-one or out-of-bounds errors.

This also affects dft_plus_u = 1 workflows. Although the main U-potential/Hamiltonian path for dft_plus_u = 1 goes through module_operator_lcao/DFTU<OperatorLCAO<...>>, Plus_U::output() is still called
in finish_dftu_lcao() for any nonzero dft_plus_u. So SCF -> NSCF/restart workflows using out_chg 1 followed by init_chg file, or workflows using omc / initial_onsite.dm, can still be broken.

A few other issues:

  1. The collinear diag=false output path no longer explicitly applies std::setprecision(8) << std::fixed when writing matrix values. Since onsite.dm is used as input/restart data, its numeric precision and
    format should remain stable.

  2. The new out_chg parameter is documented as controlling whether onsite.dm is written, but the implementation still checks if (!ofdftu) even when out_chg == false. This means out_chg = false can
    still lead to an error because the file stream was never opened. The check/write should probably be inside the out_chg && MY_RANK == 0 block.

  3. In MPI runs, only rank 0 opens onsite.dm, but all ranks appear to execute the if (!ofdftu) check. Non-root ranks may therefore see an unopened stream and report failure. The file-open check and write
    should be rank-0-only.

  4. dftu_lcao.h now uses std::string in the function signature but does not include <string> directly. It may compile through transitive includes, but the header should be self-contained.

  5. Some comments are now stale or incomplete. For example, init_dftu_lcao() still documents @param inp, while the parameter was changed to dft_plus_u; finish_dftu_lcao() adds global_out_dir, nspin,
    and npol but does not document them.

Suggested tests:

  • dft_plus_u = 1, out_chg = 1, then a following calculation with init_chg file
  • nspin = 1, nspin = 2, and nspin = 4
  • omc > 0 with initial_onsite.dm
  • out_chg = 0 should not fail due to an unopened onsite.dm
  • MPI run where only rank 0 writes onsite.dm

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