Skip to content

Commit 4397300

Browse files
mohanchenabacus_fixer
andauthored
DFT+U refactor, step 7 (#7909)
* feat(estate): add OccMatMixer skeleton for DFT+U occupation mixing 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. * refactor(pwdft): attach OccMatMixer to Plus_U_Base (transitional) 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. * refactor(pwdft): route cal_occ_pw mixing through OccMatMixer 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(). * refactor(estate): switch PW uom allocation to OccMatMixer flat_size 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. * refactor(pwdft): drop dead uom_array/uom_save/is_mixing_enabled 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. * refactor(dftu): route LCAO plain mixing through OccMatMixer::mix_plain 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. * refactor(pwdft): remove mixing_dftu member and dead enable_mixing calls 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. * add ut for occ_mixer * fix(dftu): serialize occ_save in write_save_to_flat; fix test double-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). * build(Makefile): add occ_mixer.o to OBJS_ELECSTAT 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. * fix(base): update test call sites to world_comm_domain() PR #7888 tests were added before PR #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. --------- Co-authored-by: abacus_fixer <mohanchen@pku.eud.cn>
1 parent 912824c commit 4397300

17 files changed

Lines changed: 450 additions & 46 deletions

File tree

‎source/Makefile.Objects‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,8 @@ OBJS_ELECSTAT=elecstate.o\
283283
read_orb.o\
284284
setup_estate_pw.o\
285285
update_pot.o\
286-
occ_matrix.o
286+
occ_matrix.o\
287+
occ_mixer.o
287288

288289
OBJS_ELECSTAT_LCAO=elecstate_lcao.o\
289290
init_dm.o\

‎source/source_base/test_parallel/parallel_device_test.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ TEST(ParallelDevice, CoversGpuStagingWithoutAccelerator)
124124

125125
TEST(ParallelDevice, CoversMpiTypeOverloads)
126126
{
127-
const ModuleBase::CommunicationDomain domain = ModuleBase::world_communication_domain();
127+
const ModuleBase::CommunicationDomain domain = ModuleBase::world_comm_domain();
128128
exercise_mpi_wrappers<float>(domain);
129129
exercise_mpi_wrappers<double>(domain);
130130
exercise_mpi_wrappers<std::complex<float>>(domain);

‎source/source_base/test_parallel/parallel_domain_grid_test.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ TEST(CommunicationDomainTest, ReportsDefaultAndWorldDomains)
1414
EXPECT_EQ(local_domain.rank(), 0);
1515
EXPECT_EQ(local_domain.communicator(), MPI_COMM_NULL);
1616

17-
const ModuleBase::CommunicationDomain world_domain = ModuleBase::world_communication_domain();
17+
const ModuleBase::CommunicationDomain world_domain = ModuleBase::world_comm_domain();
1818
MPICommGroup world_group(world_domain.communicator());
1919
EXPECT_EQ(world_domain.communicator(), MPI_COMM_WORLD);
2020
EXPECT_GE(world_domain.rank(), 0);
@@ -28,7 +28,7 @@ TEST(CommunicationDomainTest, ReportsDefaultAndWorldDomains)
2828

2929
TEST(MPICommGroupTest, DividesWorldIntoEvenGroups)
3030
{
31-
const ModuleBase::CommunicationDomain world_domain = ModuleBase::world_communication_domain();
31+
const ModuleBase::CommunicationDomain world_domain = ModuleBase::world_comm_domain();
3232
MPICommGroup group(MPI_COMM_WORLD);
3333
EXPECT_EQ(group.grank, world_domain.rank());
3434

@@ -46,7 +46,7 @@ TEST(MPICommGroupTest, DividesWorldIntoEvenGroups)
4646

4747
TEST(ParallelGridTest, BroadcastsAndReducesDistributedGrid)
4848
{
49-
const ModuleBase::CommunicationDomain world_domain = ModuleBase::world_communication_domain();
49+
const ModuleBase::CommunicationDomain world_domain = ModuleBase::world_comm_domain();
5050
MPICommGroup world_group(world_domain.communicator());
5151
const int nx = 2;
5252
const int ny = 1;

‎source/source_base/test_parallel/test_para_gemm.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ void expect_near_value(const T& actual, const T& expected)
7474
template <typename T>
7575
void test_additional_type_paths()
7676
{
77-
const ModuleBase::CommunicationDomain domain = ModuleBase::world_communication_domain();
77+
const ModuleBase::CommunicationDomain domain = ModuleBase::world_comm_domain();
7878
MPI_Comm world = domain.communicator();
7979
const int rank = domain.rank();
8080
MPICommGroup world_group(world);

‎source/source_estate/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ list(APPEND objects
4646
fp_energy.cpp
4747
occupy.cpp
4848
occ_matrix.cpp
49+
occ_mixer.cpp
4950
param_update.cpp
5051
setup_estate_pw.cpp
5152
update_pot.cpp

‎source/source_estate/module_charge/chgmixing.cpp‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,10 @@ void module_charge::chgmixing_ks_pw(const int iter, // scf iteration number
130130
{
131131
p_chgmix->init_mixing();
132132
p_chgmix->mixing_restart_step = inp.scf_nmax + 1;
133-
if (inp.dft_plus_u && inp.mixing_dftu)
133+
if (inp.dft_plus_u && dftu.has_occ_mixer())
134134
{
135-
// enable mixing_dftu for DFT+U occupation mixing
136-
dftu.enable_mixing();
137-
// allocate memory for uom_mdata
138-
p_chgmix->allocate_mixing_uom(dftu.get_size_pot_uterm_pw());
135+
// allocate memory for uom_mdata sized to the flat occupation buffer
136+
p_chgmix->allocate_mixing_uom(dftu.occ_mixer().flat_size());
139137
}
140138
}
141139

@@ -193,11 +191,6 @@ void module_charge::chgmixing_ks_lcao(const int iter, // scf iteration number
193191
p_chgmix->mix_reset(); // init mixing
194192
p_chgmix->mixing_restart_step = inp.scf_nmax + 1;
195193
p_chgmix->mixing_restart_count = 0;
196-
// enable mixing_dftu for DFT+U occupation mixing
197-
if (inp.dft_plus_u && inp.mixing_dftu)
198-
{
199-
dftu.enable_mixing();
200-
}
201194
// this output will be removed once the feeature is stable
202195
if (dftu.get_uramping() > 0.01)
203196
{

‎source/source_estate/occ_matrix.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,27 +280,27 @@ void OccupationMatrix::write_save_to_flat(const UnitCell& cell,
280280
for (int I = 0; I < cell.atoms[T].na; I++)
281281
{
282282
const int iat = cell.itia2iat(T, I);
283-
const int size = occ_[iat][target_l][0][0].nr * occ_[iat][target_l][0][0].nc;
283+
const int size = occ_save_[iat][target_l][0][0].nr * occ_save_[iat][target_l][0][0].nc;
284284

285285
if (nspin_ == 4)
286286
{
287287
for (int mm = 0; mm < size; mm++)
288288
{
289-
uom_save[index[iat] + mm] = occ_[iat][target_l][0][0].c[mm];
289+
uom_save[index[iat] + mm] = occ_save_[iat][target_l][0][0].c[mm];
290290
}
291291
}
292292
else if (nspin_ == 1 || nspin_ == 2)
293293
{
294294
for (int mm = 0; mm < size; mm++)
295295
{
296-
uom_save[index[iat] + mm] = occ_[iat][target_l][0][0].c[mm];
296+
uom_save[index[iat] + mm] = occ_save_[iat][target_l][0][0].c[mm];
297297
}
298298
if (nspin_ == 2)
299299
{
300300
const int half_size = uom_save.size() / 2;
301301
for (int mm = 0; mm < size; mm++)
302302
{
303-
uom_save[half_size + index[iat] + mm] = occ_[iat][target_l][0][1].c[mm];
303+
uom_save[half_size + index[iat] + mm] = occ_save_[iat][target_l][0][1].c[mm];
304304
}
305305
}
306306
}

‎source/source_estate/occ_mixer.cpp‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#include "source_estate/occ_mixer.h"
2+
3+
void OccMatMixer::init(const UnitCell* cell,
4+
const std::vector<int>* orbital_corr,
5+
const std::vector<int>* flat_index,
6+
const int nspin,
7+
const int total_size)
8+
{
9+
this->cell_ = cell;
10+
this->orbital_corr_ = orbital_corr;
11+
this->index_ = flat_index;
12+
this->nspin_ = nspin;
13+
this->uom_.resize(total_size, 0.0);
14+
this->uom_save_.resize(total_size, 0.0);
15+
}
16+
17+
void OccMatMixer::seed_save(const OccupationMatrix& occmat)
18+
{
19+
occmat.write_save_to_flat(*this->cell_, *this->orbital_corr_,
20+
*this->index_, this->uom_save_);
21+
}
22+
23+
void OccMatMixer::begin_iter(OccupationMatrix& occmat)
24+
{
25+
// the caller has already snapshotted occ into occ_save via
26+
// OccupationMatrix::copy_to_save; here we only flatten that snapshot
27+
// into uom_save_ for the mixing history.
28+
occmat.write_save_to_flat(*this->cell_, *this->orbital_corr_,
29+
*this->index_, this->uom_save_);
30+
}
31+
32+
void OccMatMixer::collect(const OccupationMatrix& occmat)
33+
{
34+
occmat.write_to_flat(*this->cell_, *this->orbital_corr_,
35+
*this->index_, this->uom_);
36+
}
37+
38+
void OccMatMixer::write_back(OccupationMatrix& occmat)
39+
{
40+
occmat.read_from_flat(*this->cell_, *this->orbital_corr_,
41+
*this->index_, this->uom_);
42+
}
43+
44+
void OccMatMixer::mix_plain(OccupationMatrix& occmat, const double beta)
45+
{
46+
elecstate::mix_occ_with_save(occmat.data(), occmat.data_save(),
47+
*this->cell_, *this->orbital_corr_,
48+
this->nspin_, beta);
49+
}

‎source/source_estate/occ_mixer.h‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
#ifndef OCC_MIXER_H
2+
#define OCC_MIXER_H
3+
4+
#include "source_estate/occ_matrix.h"
5+
6+
#include <vector>
7+
8+
class UnitCell;
9+
10+
/**
11+
* @brief Mixing of the DFT+U on-site occupation matrix.
12+
*
13+
* Owns the flattened occupation-matrix buffers used by the charge-mixing
14+
* machinery (PW path) and the plain linear mixing kernel (LCAO path).
15+
*
16+
* The flat layout reuses the pot_uterm_pw_index offset table: for nspin=2
17+
* the buffer is split into [all_up | all_dn] halves; for nspin=1/4 a single
18+
* block per atom is used. Serialization to/from the nested OccupationMatrix
19+
* is delegated to OccupationMatrix::{write_to_flat, read_from_flat,
20+
* write_save_to_flat}.
21+
*
22+
* An OccMatMixer instance exists only when mixing is enabled, so its
23+
* presence doubles as the "mixing on" flag (no mutable workflow switch).
24+
*
25+
* This class deliberately does NOT call Charge_Mixing itself; it only owns
26+
* the flat buffers and exposes them via uom()/uom_save(). The caller (the
27+
* PW driver, which already links charge_mixing) feeds these buffers to
28+
* Charge_Mixing::mix_uom and then calls write_back(). Keeping Charge_Mixing
29+
* out of this translation unit avoids dragging the planewave/xc dependency
30+
* chain into lightweight unit tests.
31+
*/
32+
class OccMatMixer
33+
{
34+
public:
35+
OccMatMixer() = default;
36+
~OccMatMixer() = default;
37+
38+
/**
39+
* @brief Allocate the flat buffers and bind the layout table.
40+
* @param cell unit cell (borrowed, must outlive this object)
41+
* @param orbital_corr per-type correlated-l table (borrowed)
42+
* @param flat_index per-atom offset table, i.e. pot_uterm_pw_index (borrowed)
43+
* @param nspin spin channels (1, 2 or 4)
44+
* @param total_size total flat-buffer size (== pot_uterm_pw.size())
45+
*/
46+
void init(const UnitCell* cell,
47+
const std::vector<int>* orbital_corr,
48+
const std::vector<int>* flat_index,
49+
int nspin,
50+
int total_size);
51+
52+
/**
53+
* @brief Seed uom_save from an occupation matrix loaded from file.
54+
*
55+
* Used when occ_mat_ctrl != 0 (restart from dm_onsite_ini.txt) so that
56+
* the first mixing step has a meaningful "previous" matrix.
57+
*/
58+
void seed_save(const OccupationMatrix& occmat);
59+
60+
/**
61+
* @brief Begin an SCF iteration: flatten the saved occ into uom_save_.
62+
*
63+
* The caller must already have snapshotted occ into occ_save via
64+
* OccupationMatrix::copy_to_save; this only flattens that snapshot into
65+
* uom_save_ for the mixing history.
66+
*/
67+
void begin_iter(OccupationMatrix& occmat);
68+
69+
/**
70+
* @brief Flatten the freshly-computed occupation matrix into uom_.
71+
*/
72+
void collect(const OccupationMatrix& occmat);
73+
74+
/**
75+
* @brief Write the (already mixed) uom_ buffer back into the occupation
76+
* matrix. Called after the caller has run Charge_Mixing::mix_uom.
77+
*/
78+
void write_back(OccupationMatrix& occmat);
79+
80+
/**
81+
* @brief Plain linear mixing for the nested-matrix (LCAO) path:
82+
* occ = beta * occ + (1 - beta) * occ_save.
83+
*
84+
* Operates directly on the nested OccupationMatrix blocks; the flat
85+
* buffers are not used. Replaces the duplicated LCAO k/gamma call sites.
86+
*/
87+
void mix_plain(OccupationMatrix& occmat, double beta);
88+
89+
/// Total flat-buffer size (== Charge_Mixing::allocate_mixing_uom argument).
90+
int flat_size() const { return static_cast<int>(uom_.size()); }
91+
92+
/// Mutable access to the new / mixed flat buffer (fed to mix_uom).
93+
std::vector<double>& uom() { return uom_; }
94+
/// Mutable access to the previous flat buffer (fed to mix_uom).
95+
std::vector<double>& uom_save() { return uom_save_; }
96+
97+
private:
98+
std::vector<double> uom_; ///< new / mixed flat occupation matrix
99+
std::vector<double> uom_save_; ///< previous flat occupation matrix
100+
const std::vector<int>* index_ = nullptr; ///< borrowed pot_uterm_pw_index
101+
const UnitCell* cell_ = nullptr; ///< borrowed unit cell
102+
const std::vector<int>* orbital_corr_ = nullptr; ///< borrowed correlated-l table
103+
int nspin_ = 0;
104+
};
105+
106+
#endif

‎source/source_estate/test/CMakeLists.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ AddTest(
8585
SOURCES potentials_base_test.cpp
8686
)
8787

88+
AddTest(
89+
TARGET MODULE_ESTATE_occ_mixer
90+
LIBS parameter base device cell_info
91+
SOURCES test_occ_mixer.cpp ../occ_mixer.cpp ../occ_matrix.cpp
92+
)
93+
8894
AddTest(
8995
TARGET MODULE_ESTATE_potentials_new
9096
LIBS parameter base device planewave_serial

0 commit comments

Comments
 (0)