Skip to content

Commit 7cc7d50

Browse files
Critsium-xyclaude
andauthored
module_dftu: pass nspin and onsite_radius explicitly, removing the two test access hacks it caused (#7940)
* dftu_pw_test: use the case value directly instead of driving global PARAM The test parked its loop variable in the global Parameter singleton and read it straight back: PARAM.input.nspin = c.nspin; switch (PARAM.inp.nspin) { ... } const double diag_coeff = PARAM.inp.nspin == 4 ? 1.0 : 0.5; It wrote the private half (`PARAM.input`) and read the public one (`PARAM.inp`), so PARAM was serving as a scratch local -- and writing the private half is the whole reason this file carried `#define private public`. Nothing else reads it: the test's only production dependency, source_pw/module_pwdft/dftu_base_tools.cpp, contains zero PARAM references. `c.nspin` is already in scope, so use it directly. The macro and the parameter.h include it guarded are removed with it. No production code changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * module_dftu: pass nspin and onsite_radius explicitly instead of reading PARAM The DFTU LCAO operator read two INPUT values straight out of the global Parameter singleton -- `PARAM.inp.nspin` in the constructor and `PARAM.inp.onsite_radius` in `initialize_HR()`. That is why dftu_lcao_test.cpp had to write `PARAM.input`, which is private, which is why it carried `#define private public`. Thread them through instead. `initialize_HR()` is private and called only from the constructor, so it simply takes `onsite_radius` as an argument; the constructor takes both and assigns the member. `dftu_nao_op.cpp` now has zero PARAM references and no longer includes parameter.h. All five call sites are updated explicitly -- no default arguments were added: - hamilt_lcao.cpp, gamma-only and multi-k branches; - force_stress_lcao.cpp, the `tmpu` force/stress instance; - the two cases in dftu_lcao_test.cpp, which now pass the values they already set locally. These first three are the composition roots for LCAO operators, the same role relax_nsync.cpp plays in #7921: the global reads concentrate there and the leaf operator stays clean. The test's expectations are unchanged. Both cases already set nspin explicitly (1 and 2) and onsite_radius in SetUp (1.0), so nothing was relying on the Input_para defaults -- the regression that cost #7921 five suites. The fixture member `onsite_radius_test = 1.0` carries the value SetUp used to write into PARAM. `#define private public` in dftu_lcao_test.cpp is removed with the reason for it. Nothing in this commit adds an `#undef private`, and no file whose macro survives is touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 34cb3a0 commit 7cc7d50

6 files changed

Lines changed: 27 additions & 26 deletions

File tree

‎source/source_lcao/force_stress_lcao.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,9 @@ void Force_Stress_LCAO<T>::getForceStress(UnitCell& ucell,
468468
&gd,
469469
two_center_bundle.overlap_orb_onsite.get(),
470470
orb.cutoffs(),
471-
&dftu);
471+
&dftu,
472+
PARAM.inp.nspin,
473+
PARAM.inp.onsite_radius);
472474

473475
tmpu.cal_force_stress(isforce, isstress, force_u, stress_u);
474476
}

‎source/source_lcao/hamilt_lcao.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,9 @@ HamiltLCAO<TK, TR>::HamiltLCAO(const UnitCell& ucell,
241241
&grid_d,
242242
two_center_bundle.overlap_orb_onsite.get(),
243243
orb.cutoffs(),
244-
p_dftu);
244+
p_dftu,
245+
PARAM.inp.nspin,
246+
PARAM.inp.onsite_radius);
245247
}
246248
this->getOperator()->add(plus_u);
247249
}
@@ -399,7 +401,9 @@ HamiltLCAO<TK, TR>::HamiltLCAO(const UnitCell& ucell,
399401
&grid_d,
400402
two_center_bundle.overlap_orb_onsite.get(),
401403
orb.cutoffs(),
402-
p_dftu);
404+
p_dftu,
405+
PARAM.inp.nspin,
406+
PARAM.inp.onsite_radius);
403407
}
404408
this->getOperator()->add(plus_u);
405409
}

‎source/source_lcao/module_dftu/dftu_nao_op.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
#include "source_base/tool_title.h"
55
#include "source_cell/module_neighbor/sltk_grid_driver.h"
66
#include "source_lcao/module_operator_lcao/operator_lcao.h"
7-
#include "source_io/module_parameter/parameter.h"
87
#include "source_base/parallel_reduce.h"
98

109
// Include the free function implementations for force/stress in real space
@@ -18,7 +17,9 @@ hamilt::DFTU<hamilt::OperatorLCAO<TK, TR>>::DFTU(HS_Matrix_K<TK>* hsk_in,
1817
const Grid_Driver* GridD_in,
1918
const TwoCenterIntegrator* intor,
2019
const std::vector<double>& orb_cutoff,
21-
Plus_U* p_dftu)
20+
Plus_U* p_dftu,
21+
const int nspin_in,
22+
const double onsite_radius)
2223
: hamilt::OperatorLCAO<TK, TR>(hsk_in, kvec_d_in, hR_in), intor_(intor), orb_cutoff_(orb_cutoff)
2324
{
2425
this->cal_type = calculation_type::lcao_dftu;
@@ -28,9 +29,9 @@ hamilt::DFTU<hamilt::OperatorLCAO<TK, TR>>::DFTU(HS_Matrix_K<TK>* hsk_in,
2829
assert(this->ucell != nullptr);
2930
#endif
3031
// initialize HR to allocate sparse Nonlocal matrix memory
31-
this->initialize_HR(GridD_in);
32+
this->initialize_HR(GridD_in, onsite_radius);
3233
// set nspin
33-
this->nspin = PARAM.inp.nspin;
34+
this->nspin = nspin_in;
3435
}
3536

3637
// destructor
@@ -41,7 +42,7 @@ hamilt::DFTU<hamilt::OperatorLCAO<TK, TR>>::~DFTU()
4142

4243
// initialize_HR()
4344
template <typename TK, typename TR>
44-
void hamilt::DFTU<hamilt::OperatorLCAO<TK, TR>>::initialize_HR(const Grid_Driver* GridD)
45+
void hamilt::DFTU<hamilt::OperatorLCAO<TK, TR>>::initialize_HR(const Grid_Driver* GridD, const double onsite_radius)
4546
{
4647
ModuleBase::TITLE("DFTU", "initialize_HR");
4748
ModuleBase::timer::start("DFTU", "initialize_HR");
@@ -75,7 +76,7 @@ void hamilt::DFTU<hamilt::OperatorLCAO<TK, TR>>::initialize_HR(const Grid_Driver
7576
// When equal, the theoretical value of matrix element is zero,
7677
// but the calculated value is not zero due to the numerical error, which would lead to result changes.
7778
if (this->ucell->cal_dtau(iat0, iat1, R_index1).norm() * this->ucell->lat0
78-
< orb_cutoff_[T1] + PARAM.inp.onsite_radius)
79+
< orb_cutoff_[T1] + onsite_radius)
7980
{
8081
is_adj[ad1] = true;
8182
}

‎source/source_lcao/module_dftu/dftu_nao_op.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ class DFTU<OperatorLCAO<TK, TR>> : public OperatorLCAO<TK, TR>
4545
const Grid_Driver* gridD_in,
4646
const TwoCenterIntegrator* intor,
4747
const std::vector<double>& orb_cutoff,
48-
Plus_U* p_dftu);
48+
Plus_U* p_dftu,
49+
const int nspin_in,
50+
const double onsite_radius);
4951
~DFTU<OperatorLCAO<TK, TR>>();
5052

5153
/**
@@ -93,7 +95,7 @@ class DFTU<OperatorLCAO<TK, TR>> : public OperatorLCAO<TK, TR>
9395
* the size of HR will not change in DFTU,
9496
* because I don't want to expand HR larger than Nonlocal operator caused by DFTU
9597
*/
96-
void initialize_HR(const Grid_Driver* gridD_in);
98+
void initialize_HR(const Grid_Driver* gridD_in, const double onsite_radius);
9799

98100
/**
99101
* @brief calculate the <phi|alpha^I> overlap values and save them in this->nlm_tot

‎source/source_lcao/module_dftu/test/dftu_lcao_test.cpp‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22
#include <chrono>
33

44
// mock of DFTU
5-
#define private public
6-
#include "source_io/module_parameter/parameter.h"
7-
#undef private
85
#include "../dftu_nao_op.h"
96
#include "source_lcao/module_dftu/dftu_nao.h"
107

@@ -99,8 +96,6 @@ class DFTUTest : public ::testing::Test
9996
}
10097
dftu.u_current = {U_test};
10198
dftu.orbital_corr = {orbital_c_test};
102-
103-
PARAM.input.onsite_radius = 1.0;
10499
}
105100

106101
void TearDown() override
@@ -147,13 +142,14 @@ class DFTUTest : public ::testing::Test
147142
int my_rank = 0;
148143
double U_test = 1.0;
149144
int orbital_c_test = 2;
145+
double onsite_radius_test = 1.0;
150146
};
151147

152148
// using TEST_F to test DFTU
153149
TEST_F(DFTUTest, constructHRd2d)
154150
{
155151
// test for nspin=1
156-
PARAM.input.nspin = 1;
152+
const int nspin = 1;
157153
std::vector<ModuleBase::Vector3<double>> kvec_d_in(1, ModuleBase::Vector3<double>(0.0, 0.0, 0.0));
158154
hamilt::HS_Matrix_K<double> hsk(paraV, true);
159155
hsk.set_zero_hk();
@@ -167,7 +163,7 @@ TEST_F(DFTUTest, constructHRd2d)
167163
}
168164
std::chrono::high_resolution_clock::time_point start_time = std::chrono::high_resolution_clock::now();
169165
hamilt::DFTU<hamilt::OperatorLCAO<double, double>>
170-
op(&hsk, kvec_d_in, HR, ucell, &gd, &intor_, {1.0}, &dftu);
166+
op(&hsk, kvec_d_in, HR, ucell, &gd, &intor_, {1.0}, &dftu, nspin, onsite_radius_test);
171167
std::chrono::high_resolution_clock::time_point end_time = std::chrono::high_resolution_clock::now();
172168
std::chrono::duration<double> elapsed_time
173169
= std::chrono::duration_cast<std::chrono::duration<double>>(end_time - start_time);
@@ -219,7 +215,7 @@ TEST_F(DFTUTest, constructHRd2d)
219215
TEST_F(DFTUTest, constructHRd2cd)
220216
{
221217
// test for nspin=2
222-
PARAM.input.nspin = 2;
218+
const int nspin = 2;
223219
std::vector<ModuleBase::Vector3<double>> kvec_d_in(2, ModuleBase::Vector3<double>(0.0, 0.0, 0.0));
224220
hamilt::HS_Matrix_K<std::complex<double>> hsk(paraV, true);
225221
hsk.set_zero_hk();
@@ -232,7 +228,7 @@ TEST_F(DFTUTest, constructHRd2cd)
232228
HR->get_wrapper()[i] = 0.0;
233229
}
234230
hamilt::DFTU<hamilt::OperatorLCAO<std::complex<double>, double>>
235-
op(&hsk, kvec_d_in, HR, ucell, &gd, &intor_, {1.0}, &dftu);
231+
op(&hsk, kvec_d_in, HR, ucell, &gd, &intor_, {1.0}, &dftu, nspin, onsite_radius_test);
236232
op.contributeHR();
237233
// check the occupations of dftu for spin-up
238234
for (int iat = 0; iat < test_size; iat++)

‎source/source_lcao/module_dftu/test/dftu_pw_test.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
#include "gtest/gtest.h"
22
#include <complex>
33
#include <vector>
4-
#define private public
5-
#include "source_io/module_parameter/parameter.h"
6-
#undef private
74
#include "source_base/matrix.h"
85
#include "source_pw/module_pwdft/dftu_base_tools.h"
96

@@ -40,15 +37,14 @@ TEST_F(DftuPwTest, EnergyWeightsAllNspin)
4037
struct Case { int nspin; double expected_weight; double expected_diag; };
4138
Case cases[] = {{1, 1.0, 0.5}, {2, 0.5, 0.5}, {4, 0.25, 1.0}};
4239
for (const auto& c : cases) {
43-
PARAM.input.nspin = c.nspin;
4440
double weight_eu = 1;
45-
switch (PARAM.inp.nspin) {
41+
switch (c.nspin) {
4642
case 1: weight_eu = 1.0; break;
4743
case 2: weight_eu = 0.5; break;
4844
case 4: weight_eu = 0.25; break;
4945
default: break;
5046
}
51-
const double diag_coeff = PARAM.inp.nspin == 4 ? 1.0 : 0.5;
47+
const double diag_coeff = c.nspin == 4 ? 1.0 : 0.5;
5248
EXPECT_DOUBLE_EQ(weight_eu, c.expected_weight);
5349
EXPECT_DOUBLE_EQ(diag_coeff, c.expected_diag);
5450
}

0 commit comments

Comments
 (0)