Skip to content

DFT+U refactor, step 7 - #7909

Open
mohanchen wants to merge 13 commits into
deepmodeling:developfrom
mohanchen:2026-09-04-line2
Open

DFT+U refactor, step 7#7909
mohanchen wants to merge 13 commits into
deepmodeling:developfrom
mohanchen:2026-09-04-line2

Conversation

@mohanchen

Copy link
Copy Markdown
Collaborator

DFT+U refactor, step 7

abacus_fixer added 8 commits September 4, 2026 13:08
Introduce OccMatMixer in source_estate to own the flattened
occupation-matrix buffers (uom/uom_save) currently embedded in
Plus_U_Base. Step 1 only adds the class skeleton (init/seed_save/
begin_iter/collect/mix) and wires occ_mixer.cpp into CMake; the
migration of state and call sites follows in subsequent commits.
Compose Plus_U_Base with a std::unique_ptr<OccMatMixer>, constructed in
init_base only when mixing_dftu != 0 so that "mixer exists" doubles as
the mixing-on flag. The legacy uom_array/uom_save members and the
is_mixing_enabled/enable_mixing accessors are kept for now; they are
removed in a later commit once all call sites migrate.

Invert the Charge_Mixing dependency: OccMatMixer no longer calls
Charge_Mixing::mix_uom itself. It only owns the flat buffers, exposes
them via uom()/uom_save(), and provides write_back() to deserialize the
mixed result. The PW driver (which already links charge_mixing) feeds
the buffers to mix_uom. This keeps occ_mixer.cpp dependent solely on
occ_matrix.h, so the PW and LCAO dftu unit tests link with just
occ_matrix.cpp + occ_mixer.cpp instead of dragging in the planewave/xc
dependency chain (PW_Basis::real2recip, XC_Functional::ked_flag).

Wire occ_mixer.cpp into the dftu_base_test and dftu_lcao_test sources.
cal_occ_pw now drives the PW mixing orchestration via the composed
OccMatMixer instead of the legacy uom_array/uom_save members:
begin_iter flattens the saved occ into uom_save, collect flattens the
fresh occ into uom, the buffers are fed to Charge_Mixing::mix_uom, and
write_back deserializes the mixed result. The enable condition switches
from is_mixing_enabled() to has_occ_mixer().

The flat save/new writes now run only when a mixer exists; they were
previously executed unconditionally but their results were only read
inside the mixing branch, so behavior is bit-identical. uom_array and
uom_save are now dead members and are removed in a later commit.

OccMatMixer::begin_iter no longer performs copy_to_save itself; that
snapshot stays in cal_occ_pw because it must precede occmat_.zero().
In chgmixing_ks_pw, drop the now-dead dftu.enable_mixing() call (the
mixer is constructed in init_base and cal_occ_pw reads has_occ_mixer()),
gate the uom_mdata allocation on has_occ_mixer() instead of
inp.mixing_dftu, and size it with occ_mixer().flat_size() which equals
the previous get_size_pot_uterm_pw(). The LCAO branch's dead
enable_mixing() call is left for the PARAM-dependency cleanup step.
Remove the flat occupation buffers uom_array/uom_save from Plus_U_Base
(now owned by OccMatMixer) together with their resize in init_base, and
the is_mixing_enabled() accessor that has no callers left. enable_mixing()
and the mixing_dftu member stay for now because the LCAO path still calls
enable_mixing(); they are removed alongside the PARAM.inp.mixing_dftu
dependency in the cleanup step.
Add OccMatMixer::mix_plain(occmat, beta), which delegates to
elecstate::mix_occ_with_save on the nested occupation blocks (the flat
buffers are PW/Broyden-only). Replace the two duplicated LCAO k/gamma
call sites with dftu.occ_mixer().mix_plain(dftu.occmat(), mixing_beta)
and gate them on has_occ_mixer() instead of PARAM.inp.mixing_dftu,
removing that cross-layer global read from dftu_nao_occ.cpp.
The mutable workflow switch is fully retired: drop the LCAO branch's dead
dftu.enable_mixing() block in chgmixing_ks_lcao (the flag it set was never
read), the enable_mixing() accessor, and the mixing_dftu member of
Plus_U_Base. The init_base mixing_dftu parameter is kept as the sole
decision input for constructing the OccMatMixer; presence of the mixer is
now the only "mixing on" signal.
@mohanchen
mohanchen requested a review from lanshuyue September 4, 2026 09:31
@mohanchen mohanchen added DFT+U Issues related to DFT plus U function Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0 labels Sep 4, 2026
abacus_fixer and others added 5 commits September 4, 2026 17:36
Resolve conflicts in dftu_base.h and dftu_base_occ.cpp by keeping local
refactor (use has_occ_mixer() instead of removed mixing_dftu member).
…free

OccupationMatrix::write_save_to_flat is documented to flatten occ_save_
into the uom_save mixing buffer, but it was reading occ_ instead, so the
first mixing step after seed_save/begin_iter used the wrong history
matrix.

In test_occ_mixer, Statistics::~Statistics() owns and delete[]s
iat2it/iat2ia, so backing them with std::vector storage caused a
double-free. Allocate them with new[] instead and drop the extra
backing vectors.

Verified: cmake --build build --target MODULE_ESTATE_occ_mixer &&
OMP_NUM_THREADS=1 ./build/source/source_estate/test/MODULE_ESTATE_occ_mixer
(4/4 tests pass).
The Makefile build links dftu_base_occ.o/dftu_nao_occ.o against
OccMatMixer, but occ_mixer.cpp was missing from Makefile.Objects,
causing undefined-reference link errors for write_back/mix_plain.
CMake already wires it via source_estate/CMakeLists.txt.
PR deepmodeling#7888 tests were added before PR deepmodeling#7900 renamed
ModuleBase::world_communication_domain() to
ModuleBase::world_comm_domain(); update the three affected
test files so the parallel test targets build again.

Verified: cmake --build build --target
MODULE_BASE_parallel_domain_grid MODULE_BASE_para_gemm
MODULE_BASE_parallel_device all succeed.
@AsTonyshment AsTonyshment linked an issue Sep 5, 2026 that may be closed by this pull request
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 The Absolute Zero Reduce the "entropy" of the code to 0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build fails after merging PR #7888 and PR #7900

1 participant