Skip to content

Commit 31c899d

Browse files
mohanchenabacus_fixer
andauthored
Update source_cell module and agent governance (deepmodeling#7625)
* resolve the wrong dependency between klist and berryphase * remove useless things * replace ucell.lc with vector<int> * change the name of a variable * remove dependency of PARAM * move test of magnetism * update magnetism * update agent governance check * update agent governance check * refactor(cell): centralize Magnetism::start_mag initialization in setup() and read_atom_positions() ### Why Previously `Magnetism::start_mag` had no well-defined single owner for memory allocation. Even though `UnitCell::setup_cell()` happened to resize it, any caller that invoked `unitcell::read_atom_positions()` directly (as most unit tests did) was required to *externally* call `ucell->magnet.start_mag.resize(ucell->ntype)` first. This undocumented implicit contract produced dozens of scattered, copy-paste pre-conditions throughout the test harness and made the API easy to misuse. ### What 1. Proactive initialization in `UnitCell::setup()` - As soon as `ntype` is set, resize `start_mag` to `ntype_in` with value 0.0. - This covers every test helper that already goes through `setup()` so the callers no longer need to manually size and zero-initialize the vector. 2. Defensive auto-resize inside `unitcell::read_atom_positions()` - Before any access to `start_mag[it]`, check that its size matches `ucell.ntype` and resize if needed. - This guards the remaining code paths that set `ntype` by hand and guarantees the consumer owns its pre-conditions. 3. Removed the now-redundant explicit resize in `UnitCell::setup_cell()`. Kept the `assert(ntype > 0)` validity check; renumbered the following step comment from `(2)` to `(1)`. 4. Cleaned up ~20+ copies of the redundant preparatory lines across the test suites: - 7 `prepare_unitcell.h` helpers (source_cell, source_cell/module_neighbor, source_estate, source_estate/module_dm, source_lcao/module_hcontainer, source_io, source_io/test_serial): removed `ucell->magnet.start_mag.resize(ntype)` and the loop body `start_mag[it] = 0.0`. - Unit tests `unitcell_test.cpp`, `unitcell_test_pw.cpp` and `unitcell_test_setupcell.cpp`: removed the now-unnecessary "mandatory preliminaries" resize block before `read_atom_positions`. ### Files touched (12) source/source_cell/unitcell.cpp source/source_cell/read_atoms.cpp source/source_cell/module_neighbor/test/prepare_unitcell.h source/source_cell/test/prepare_unitcell.h source/source_cell/test/unitcell_test.cpp source/source_cell/test/unitcell_test_setupcell.cpp source/source_cell/test_pw/unitcell_test_pw.cpp source/source_estate/test/prepare_unitcell.h source/source_estate/module_dm/test/prepare_unitcell.h source/source_io/test/prepare_unitcell.h source/source_io/test_serial/prepare_unitcell.h source/source_lcao/module_hcontainer/test/prepare_unitcell.h Behavior change: none; resize is idempotent and every removed site defaulted to the same zero value the new helpers already use. * fix bugs --------- Co-authored-by: abacus_fixer <mohanchen@pku.eud.cn>
1 parent 1a0d849 commit 31c899d

49 files changed

Lines changed: 362 additions & 410 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

source/source_cell/atom_spec.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class Atom
3131
std::vector<int> l_nchi; // number of chi for each L
3232
int stapos_wf = 0; // start position of wave functions
3333

34-
std::string label = "\0"; // atomic symbol
34+
std::string label; ///< atomic symbol
3535
std::vector<ModuleBase::Vector3<double>> tau; // Cartesian coordinates of each atom in this type.
3636
std::vector<ModuleBase::Vector3<double>> dis; // direct displacements of each atom in this type in current step liuyu modift 2023-03-22
3737
std::vector<ModuleBase::Vector3<double>> taud; // Direct coordinates of each atom in this type.
@@ -40,7 +40,7 @@ class Atom
4040
std::vector<ModuleBase::Vector3<double>> force; // force acting on each atom in this type.
4141
std::vector<ModuleBase::Vector3<double>> lambda; // Lagrange multiplier for each atom in this type. used in deltaspin
4242
std::vector<ModuleBase::Vector3<int>> constrain; // constrain for each atom in this type. used in deltaspin
43-
std::string label_orb = "\0"; // atomic Element symbol in the orbital file of lcao
43+
std::string label_orb; ///< atomic element symbol in the orbital file of lcao
4444

4545
std::vector<double> mag;
4646
std::vector<double> angle1; // spin angle, added by zhengdy-soc

source/source_cell/bcast_cell.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ namespace unitcell
8181
Parallel_Common::bcast_double(lat.a2[i]);
8282
Parallel_Common::bcast_double(lat.a3[i]);
8383
Parallel_Common::bcast_double(lat.latcenter[i]);
84-
Parallel_Common::bcast_int(lat.lc[i]);
84+
Parallel_Common::bcast_int(lat.lat_axis_free[i]);
8585
}
8686

8787
// distribute superlattice vectors.
@@ -103,7 +103,11 @@ namespace unitcell
103103
{
104104
#ifdef __MPI
105105
MPI_Barrier(MPI_COMM_WORLD);
106-
Parallel_Common::bcast_double(magnet.start_mag, ntype);
106+
if (GlobalV::MY_RANK != 0)
107+
{
108+
magnet.start_mag.resize(ntype, 0.0);
109+
}
110+
Parallel_Common::bcast_double(magnet.start_mag.data(), ntype);
107111
if (PARAM.inp.nspin == 4)
108112
{
109113
Parallel_Common::bcast_double(magnet.ux_[0]);

source/source_cell/magnetism.cpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
#include "magnetism.h"
2-
32
#include "source_base/parallel_reduce.h"
4-
#include "source_io/module_parameter/parameter.h"
5-
//#include "source_estate/module_charge/charge.h"
63

74
Magnetism::Magnetism()
85
{
@@ -14,21 +11,23 @@ Magnetism::Magnetism()
1411

1512
Magnetism::~Magnetism()
1613
{
17-
delete[] start_mag;
1814
}
1915

2016
void Magnetism::compute_mag(const double& omega,
21-
const int& nrxx,
22-
const int& nxyz,
23-
const double* const * rho,
17+
const int& nrxx,
18+
const int& nxyz,
19+
const double* const * rho,
20+
const int& nspin,
21+
const bool& two_fermi,
22+
const double& nelec,
2423
double* nelec_spin)
2524
{
2625
assert(omega>0.0);
2726
assert(nxyz>0);
2827

2928
const double fac = omega / nxyz;
3029

31-
if (PARAM.inp.nspin==2)
30+
if (nspin==2)
3231
{
3332
this->tot_mag = 0.00;
3433
this->abs_mag = 0.00;
@@ -51,17 +50,17 @@ void Magnetism::compute_mag(const double& omega,
5150

5251
//update number of electrons for each spin
5352
//if TWO_EFERMI, no need to update
54-
if(!PARAM.globalv.two_fermi)
53+
if(!two_fermi)
5554
{
56-
nelec_spin[0] = (PARAM.inp.nelec + this->tot_mag) / 2;
57-
nelec_spin[1] = (PARAM.inp.nelec - this->tot_mag) / 2;
55+
nelec_spin[0] = (nelec + this->tot_mag) / 2;
56+
nelec_spin[1] = (nelec - this->tot_mag) / 2;
5857
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running,"Electron number for spin up", nelec_spin[0]);
5958
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running,"Electron number for spin down", nelec_spin[1]);
6059
}
6160
}
6261

6362
// noncolliear :
64-
else if(PARAM.inp.nspin==4)
63+
else if(nspin==4)
6564
{
6665
for(int i=0;i<3;i++)
6766
{

source/source_cell/magnetism.h

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include "source_base/global_function.h"
55
#include "source_base/vector3.h"
6+
#include <vector>
67

78
class Magnetism
89
{
@@ -12,7 +13,7 @@ class Magnetism
1213
~Magnetism();
1314

1415
// notice : bcast (MPI operation) is done in unitcell
15-
double *start_mag=nullptr;
16+
std::vector<double> start_mag;
1617

1718
// tot_mag : majority spin - minority spin (nelup - neldw).
1819
double tot_mag;
@@ -22,16 +23,13 @@ class Magnetism
2223
double abs_mag;
2324

2425
void compute_mag(const double& omega,
25-
const int& nrxx,
26-
const int& nxyz,
27-
const double* const * rho,
28-
double* nelec_spin = nullptr);
29-
30-
ModuleBase::Vector3<double> *m_loc_=nullptr; //magnetization for each element along c-axis
31-
32-
double *angle1_=nullptr; //angle between c-axis and real spin std::vector
33-
34-
double *angle2_=nullptr; //angle between a-axis and real spin std::vector projection in ab-plane
26+
const int& nrxx,
27+
const int& nxyz,
28+
const double* const * rho,
29+
const int& nspin,
30+
const bool& two_fermi,
31+
const double& nelec,
32+
double* nelec_spin);
3533

3634
double ux_[3]={0.0};
3735

source/source_cell/module_neighbor/test/prepare_unitcell.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,11 @@ class UcellTestPrepare
7777
this->init_vel,
7878
this->fixed_axes);
7979

80-
delete[] ucell->magnet.start_mag; //mag set here
8180
ucell->atom_label.resize(ucell->ntype);
8281
ucell->atom_mass.resize(ucell->ntype);
8382
ucell->pseudo_fn.resize(ucell->ntype);
8483
ucell->pseudo_type.resize(ucell->ntype);
8584
ucell->orbital_fn.resize(ucell->ntype);
86-
ucell->magnet.start_mag = new double[ucell->ntype]; //mag set here
8785
ucell->magnet.ux_[0] = 0.0; // ux_ set here
8886
ucell->magnet.ux_[1] = 0.0;
8987
ucell->magnet.ux_[2] = 0.0;
@@ -94,7 +92,6 @@ class UcellTestPrepare
9492
ucell->pseudo_fn[it] = this->pp_files[it];
9593
ucell->pseudo_type[it] = this->pp_types[it];
9694
ucell->orbital_fn[it] = this->orb_files[it];
97-
ucell->magnet.start_mag[it] = 0.0; //mag set here
9895
}
9996
//lattice info
10097
ucell->lat0 = this->lat0;

source/source_cell/module_neighbor/test/sltk_atom_arrange_test.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,9 @@ Magnetism::Magnetism()
2828
{
2929
this->tot_mag = 0.0;
3030
this->abs_mag = 0.0;
31-
this->start_mag = nullptr;
3231
}
3332
Magnetism::~Magnetism()
3433
{
35-
delete[] this->start_mag;
3634
}
3735

3836
/************************************************

source/source_cell/module_neighbor/test/sltk_grid_test.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,9 @@ Magnetism::Magnetism()
2525
{
2626
this->tot_mag = 0.0;
2727
this->abs_mag = 0.0;
28-
this->start_mag = nullptr;
2928
}
3029
Magnetism::~Magnetism()
3130
{
32-
delete[] this->start_mag;
3331
}
3432

3533
/************************************************

source/source_cell/read_atoms.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ bool unitcell::read_atom_positions(UnitCell& ucell,
2525
const int nspin = PARAM.inp.nspin;
2626
assert (nspin==1 || nspin==2 || nspin==4);
2727

28+
if (ucell.magnet.start_mag.size() != static_cast<size_t>(ntype))
29+
{
30+
ucell.magnet.start_mag.resize(ntype, 0.0);
31+
}
32+
2833
if( ModuleBase::GlobalFunc::SCAN_LINE_BEGIN(ifpos, "ATOMIC_POSITIONS"))
2934
{
3035
ModuleBase::GlobalFunc::READ_VALUE(ifpos, Coordinate);

source/source_cell/test/CMakeLists.txt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ abacus_disable_feature_definitions(__ROCM)
44
abacus_disable_feature_definitions(__EXX)
55

66
find_program(BASH bash)
7+
file(COPY support DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
8+
configure_file(bcast_atom_pseudo_test.sh ${CMAKE_CURRENT_BINARY_DIR}/bcast_atom_pseudo_test.sh COPYONLY)
9+
configure_file(bcast_atom_spec_test.sh ${CMAKE_CURRENT_BINARY_DIR}/bcast_atom_spec_test.sh COPYONLY)
10+
configure_file(parallel_kpoints_test.sh ${CMAKE_CURRENT_BINARY_DIR}/parallel_kpoints_test.sh COPYONLY)
11+
configure_file(klist_test_para.sh ${CMAKE_CURRENT_BINARY_DIR}/klist_test_para.sh COPYONLY)
12+
configure_file(unitcell_test_parallel.sh ${CMAKE_CURRENT_BINARY_DIR}/unitcell_test_parallel.sh COPYONLY)
13+
configure_file(bcast_read_sep_test.sh ${CMAKE_CURRENT_BINARY_DIR}/bcast_read_sep_test.sh COPYONLY)
14+
configure_file(bcast_sep_cell_test.sh ${CMAKE_CURRENT_BINARY_DIR}/bcast_sep_cell_test.sh COPYONLY)
715
install(DIRECTORY support DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
816
install(FILES bcast_atom_pseudo_test.sh DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
917
install(FILES bcast_atom_spec_test.sh DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
@@ -193,3 +201,9 @@ add_test(NAME MODULE_CELL_sep_cell_parallel
193201
COMMAND ${BASH} bcast_sep_cell_test.sh
194202
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
195203
)
204+
205+
AddTest(
206+
TARGET MODULE_CELL_magnetism
207+
LIBS base device
208+
SOURCES magnetism_test.cpp ../magnetism.cpp
209+
)

source/source_estate/test/elecstate_magnetism_test.cpp renamed to source/source_cell/test/magnetism_test.cpp

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@
66
// mohan add 2025-04-12
77
#include "source_estate/module_charge/charge.h"
88

9-
#define private public
10-
#include "source_io/module_parameter/parameter.h"
11-
#undef private
12-
139
/************************************************
1410
* unit test of magnetism.cpp
1511
***********************************************/
@@ -20,8 +16,8 @@
2016
* - Magnetism::~Magnetism()
2117
* - Magnetism::judge_parallel()
2218
* - Magnetism::compute_mag()
23-
* - compute mag for spin-polarized system when PARAM.input.nspin = 2
24-
* - and non-collinear case with PARAM.input.nspin = 4
19+
* - compute mag for spin-polarized system when nspin = 2
20+
* - and non-collinear case with nspin = 4
2521
*/
2622

2723
#define private public
@@ -53,7 +49,7 @@ TEST_F(MagnetismTest, Magnetism)
5349
{
5450
EXPECT_EQ(0.0, magnetism->tot_mag);
5551
EXPECT_EQ(0.0, magnetism->abs_mag);
56-
EXPECT_EQ(nullptr, magnetism->start_mag);
52+
EXPECT_TRUE(magnetism->start_mag.empty());
5753
}
5854

5955
TEST_F(MagnetismTest, JudgeParallel)
@@ -67,15 +63,15 @@ TEST_F(MagnetismTest, JudgeParallel)
6763

6864
TEST_F(MagnetismTest, ComputeMagnetizationS2)
6965
{
70-
PARAM.input.nspin = 2;
71-
PARAM.sys.two_fermi = false;
72-
PARAM.input.nelec = 10.0;
66+
const int nspin = 2;
67+
const bool two_fermi = false;
68+
const double nelec = 10.0;
7369

7470
Charge* chr = new Charge;
7571
chr->nrxx = 100;
7672
chr->nxyz = 1000;
77-
chr->rho = new double*[PARAM.input.nspin];
78-
for (int i=0; i< PARAM.input.nspin; i++)
73+
chr->rho = new double*[nspin];
74+
for (int i=0; i< nspin; i++)
7975
{
8076
chr->rho[i] = new double[chr->nrxx];
8177
}
@@ -85,13 +81,14 @@ TEST_F(MagnetismTest, ComputeMagnetizationS2)
8581
chr->rho[1][ir] = 1.01;
8682
}
8783
double* nelec_spin = new double[2];
88-
magnetism->compute_mag(500.0,chr->nrxx, chr->nxyz, chr->rho, nelec_spin);
84+
magnetism->compute_mag(500.0,chr->nrxx, chr->nxyz, chr->rho,
85+
nspin, two_fermi, nelec, nelec_spin);
8986
EXPECT_DOUBLE_EQ(-0.5, magnetism->tot_mag);
9087
EXPECT_DOUBLE_EQ(0.5, magnetism->abs_mag);
9188
EXPECT_DOUBLE_EQ(4.75, nelec_spin[0]);
9289
EXPECT_DOUBLE_EQ(5.25, nelec_spin[1]);
9390
delete[] nelec_spin;
94-
for (int i=0; i< PARAM.input.nspin; i++)
91+
for (int i=0; i< nspin; i++)
9592
{
9693
delete[] chr->rho[i];
9794
}
@@ -101,13 +98,13 @@ TEST_F(MagnetismTest, ComputeMagnetizationS2)
10198

10299
TEST_F(MagnetismTest, ComputeMagnetizationS4)
103100
{
104-
PARAM.input.nspin = 4;
101+
const int nspin = 4;
105102

106103
Charge* chr = new Charge;
107-
chr->rho = new double*[PARAM.input.nspin];
104+
chr->rho = new double*[nspin];
108105
chr->nrxx = 100;
109106
chr->nxyz = 1000;
110-
for (int i=0; i< PARAM.input.nspin; i++)
107+
for (int i=0; i< nspin; i++)
111108
{
112109
chr->rho[i] = new double[chr->nrxx];
113110
}
@@ -119,13 +116,14 @@ TEST_F(MagnetismTest, ComputeMagnetizationS4)
119116
chr->rho[3][ir] = 1.00;
120117
}
121118
double* nelec_spin = new double[4];
122-
magnetism->compute_mag(500.0,chr->nrxx, chr->nxyz, chr->rho, nelec_spin);
119+
magnetism->compute_mag(500.0,chr->nrxx, chr->nxyz, chr->rho,
120+
nspin, false, 0.0, nelec_spin);
123121
EXPECT_DOUBLE_EQ(100.0, magnetism->abs_mag);
124122
EXPECT_DOUBLE_EQ(50.0*std::sqrt(2.0), magnetism->tot_mag_nc[0]);
125123
EXPECT_DOUBLE_EQ(50.0, magnetism->tot_mag_nc[1]);
126124
EXPECT_DOUBLE_EQ(50.0, magnetism->tot_mag_nc[2]);
127125
delete[] nelec_spin;
128-
for (int i=0; i< PARAM.input.nspin; i++)
126+
for (int i=0; i< nspin; i++)
129127
{
130128
delete[] chr->rho[i];
131129
}
@@ -135,18 +133,20 @@ TEST_F(MagnetismTest, ComputeMagnetizationS4)
135133

136134
#ifdef __MPI
137135
#include <mpi.h>
136+
#include "source_base/parallel_comm.h"
138137
int main(int argc, char **argv)
139138
{
140-
141139
MPI_Init(&argc, &argv);
142-
MPI_Comm_size(MPI_COMM_WORLD,&GlobalV::NPROC);
143-
MPI_Comm_rank(MPI_COMM_WORLD,&GlobalV::MY_RANK);
140+
MPI_Comm_size(MPI_COMM_WORLD, &GlobalV::NPROC);
141+
MPI_Comm_rank(MPI_COMM_WORLD, &GlobalV::MY_RANK);
142+
MPI_Comm_dup(MPI_COMM_WORLD, &POOL_WORLD);
144143

145144
testing::InitGoogleTest(&argc, argv);
146145
int result = RUN_ALL_TESTS();
147146

147+
MPI_Comm_free(&POOL_WORLD);
148148
MPI_Finalize();
149-
150149
return result;
151150
}
152151
#endif
152+

0 commit comments

Comments
 (0)