DFT+U Refactor#7538
Conversation
lanshuyue
left a comment
There was a problem hiding this comment.
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:
-
The collinear
diag=falseoutput path no longer explicitly appliesstd::setprecision(8) << std::fixedwhen writing matrix values. Sinceonsite.dmis used as input/restart data, its numeric precision and
format should remain stable. -
The new
out_chgparameter is documented as controlling whetheronsite.dmis written, but the implementation still checksif (!ofdftu)even whenout_chg == false. This meansout_chg = falsecan
still lead to an error because the file stream was never opened. The check/write should probably be inside theout_chg && MY_RANK == 0block. -
In MPI runs, only rank 0 opens
onsite.dm, but all ranks appear to execute theif (!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. -
dftu_lcao.hnow usesstd::stringin the function signature but does not include<string>directly. It may compile through transitive includes, but the header should be self-contained. -
Some comments are now stale or incomplete. For example,
init_dftu_lcao()still documents@param inp, while the parameter was changed todft_plus_u;finish_dftu_lcao()addsglobal_out_dir,nspin,
andnpolbut does not document them.
Suggested tests:
dft_plus_u = 1,out_chg = 1, then a following calculation withinit_chg filenspin = 1,nspin = 2, andnspin = 4omc > 0withinitial_onsite.dmout_chg = 0should not fail due to an unopenedonsite.dm- MPI run where only rank 0 writes
onsite.dm
DFT+U Refactor