DFT+U Refactor#7538
Conversation
mohanchen
added
DFT+U
lanshuyue
left a comment
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
This commit addresses several issues in the DFT+U I/O code related to the
onsite.dm file handling:
1. Format compatibility fix: Updated read_occup_m() to parse the new output format
with labels Atom=, L=, ORBITAL=, and spin= instead of the old tokens atoms, L,
zeta, and spin. The writer was already using the new format but the reader
was not updated, causing new onsite.dm files to be unreadable.
2. Off-by-one fix: The writer outputs 1-based indices (iat+1, is+1) for human
readability, but read_occup_m() was using these values directly as array
indices. Added iat -= 1 and spin -= 1 to convert back to 0-based indices.
3. Precision fix: Added std::setprecision(8) << std::fixed for the collinear
(nspin=1,2) diag=false output path in write_occup_m(). The eigenvalues path
and SOC path already had this, but the matrix values path was missing it.
This ensures stable numeric precision for restart data.
4. out_chg logic fix: The out_chg parameter is supposed to control whether
onsite.dm is written, but the implementation was still checking if(!ofdftu)
even when out_chg == false. Moved the file-open check and write operations
inside the out_chg && MY_RANK == 0 block.
5. MPI fix: Only rank 0 opens the onsite.dm file, but all ranks were executing
the if(!ofdftu) check and write_occup_m() call. Non-root ranks would see an
unopened stream and potentially fail. Now all file operations are rank-0-only.
6. Header fix: Added missing #include <string> and #include <vector> to
dftu_lcao.h. The header was using std::string and std::vector in function
signatures but relying on transitive includes.
7. Documentation fix: Updated stale comments in dftu_lcao.h:
- Changed @param inp to @param dft_plus_u in init_dftu_lcao()
- Added documentation for global_out_dir, nspin, and npol parameters in
finish_dftu_lcao()
8. Error handling: Replaced all exit(0) calls with ModuleBase::WARNING_QUIT()
for consistent error handling across the codebase.
Files modified:
- source/source_lcao/module_dftu/dftu_io.cpp
- source/source_lcao/dftu_lcao.h
- source/source_lcao/dftu_lcao.cpp
This commit renames the DFT+U occupation matrix files and adjusts their read/write paths:
1. File name changes:
- onsite.dm → dm_onsite.txt (output from DFT+U calculations)
- initial_onsite.dm → dm_onsite_ini.txt (user-provided initial occupation matrix)
2. Path adjustments:
- dm_onsite.txt: Both written and read from global_out_dir (OUT.prefix)
- Previously read from global_readin_dir
- This ensures the file is always in the output directory
- dm_onsite_ini.txt: Read from global_readin_dir (set via read_file_dir parameter)
- Previously read from global_out_dir
- This allows users to place initial files in any directory
3. Code changes:
- dftu_io.cpp: Updated output filename and error messages
- dftu.cpp: Updated read paths for both files
- dftu_lcao.h and module_operator_lcao/dftu_lcao.cpp: Updated comments
4. Documentation updates:
- read_input_item_exx_dftu.cpp: Updated omc parameter description
- parameters.yaml: Updated omc parameter description
- input-main.md: Updated omc parameter description
- band.md and dos.md: Updated file references
- tests/17_DS_DFTU/README.md: Updated test documentation
- tests/17_DS_DFTU/run_scf_nscf.sh: Updated copy logic for dm_onsite.txt
5. Backward compatibility:
- The new format with labels Atom=, L=, ORBITAL=, spin= was already in the writer
- This commit updates the reader to parse the new format
- 1-based indices in output are converted to 0-based when reading
Files modified:
- source/source_lcao/module_dftu/dftu_io.cpp
- source/source_lcao/module_dftu/dftu.cpp
- source/source_lcao/dftu_lcao.h
- source/source_lcao/module_operator_lcao/dftu_lcao.cpp
- source/source_io/module_parameter/read_input_item_exx_dftu.cpp
- docs/parameters.yaml
- docs/advanced/input_files/input-main.md
- docs/advanced/elec_properties/band.md
- docs/advanced/elec_properties/dos.md
- tests/17_DS_DFTU/README.md
- tests/17_DS_DFTU/run_scf_nscf.sh
2 participants
DFT+U Refactor