Skip to content

Commit d47dfd0

Browse files
author
abacus_fixer
committed
fix(parallel): reproduce exact legacy call order in create_partition
Two bugs caused the np=4/kpar=2 SIGSEGV in the previous attempt: 1. Call order: the factory called divide_pools before split_diag_world / split_grid_world, while the legacy driver does the reverse. Restored the exact legacy order: split_diag_world -> split_grid_world -> divide_pools. 2. GlobalV write-back: the factory passed local int variables to divide_pools, so GlobalV::NPROC_IN_POOL / RANK_IN_POOL / MY_POOL / NPROC_IN_BNDGROUP / RANK_IN_BPGROUP / MY_BNDGROUP were never updated and stayed at 0, causing downstream consumers to read garbage. Now the factory passes the GlobalV references directly, exactly as the legacy init_pools did. Also add the driver single-point switch to create_partition and the ESolver topology injection (set_topology) that were reverted in the previous step. Verified: np=4 with kpar=1/2/4, bndpar=1, diago=1/2 all pass; 4 MPI parallel unit tests all pass; 4-rank mpirun ParallelPartition 9/9.
1 parent ed0c3e4 commit d47dfd0

4 files changed

Lines changed: 72 additions & 54 deletions

File tree

source/source_base/parallel_global.cpp

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -314,28 +314,11 @@ ParallelPartition Parallel_Global::create_partition(int world_nproc,
314314
}
315315
}
316316

317-
// ---- Invoke the legacy divide_pools flow: it fills the 6 legacy
318-
// global communicators AND returns scalar int values. ----
319-
//
320-
// NOTE(mohan): order matters. divide_pools internally calls
321-
// kpar_group .divide_group_comm(KPAR, false);
322-
// bndpar_group.divide_group_comm(BNDPAR, true);
323-
// -> sets POOL_WORLD, KP_WORLD, INT_BGROUP, BP_WORLD.
324-
int nproc_in_bndgroup = -1;
325-
int rank_in_bpgroup = -1;
326-
int my_bndgroup = -1;
327-
int nproc_in_pool_local = -1; // output alias of bndpar_group.nprocs_in_group
328-
int rank_in_pool_local = -1; // output alias of bndpar_group.rank_in_group
329-
int my_pool_local = -1; // output alias of kpar_group.my_group
330-
Parallel_Global::divide_pools(world_nproc, my_rank, bndpar, kpar,
331-
nproc_in_bndgroup, rank_in_bpgroup, my_bndgroup,
332-
nproc_in_pool_local, rank_in_pool_local, my_pool_local);
333-
334-
// ---- split diag / rgrid worlds. ----
335-
// These two helpers currently write DIAG_WORLD / GRID_WORLD as a
336-
// side effect; they were always called by drivers in the old flow
337-
// right after divide_pools, so we fold them into the factory to
338-
// centralise all 6 legacy comm + scalar partition construction.
317+
// ---- Reproduce the exact legacy driver call order: ----
318+
// 1. split_diag_world (writes DIAG_WORLD)
319+
// 2. split_grid_world (writes GRID_WORLD)
320+
// 3. divide_pools (writes POOL_WORLD, KP_WORLD, INT_BGROUP, BP_WORLD)
321+
// Do NOT reorder these calls without a separate validation PR.
339322
//
340323
// diag_np == 0 is not meaningful; fall back to 1 so the
341324
// even-partition guard in divide_mpi_groups (called inside
@@ -347,16 +330,32 @@ ParallelPartition Parallel_Global::create_partition(int world_nproc,
347330
int grank = -1, gsize = -1;
348331
Parallel_Global::split_grid_world(effective_diag_np, world_nproc, my_rank, grank, gsize);
349332

333+
// ---- Invoke the legacy divide_pools flow: it fills the 6 legacy
334+
// global communicators AND writes the GlobalV scalar fields
335+
// (NPROC_IN_POOL, RANK_IN_POOL, MY_POOL, etc.). ----
336+
//
337+
// NOTE(mohan): order matters. divide_pools internally calls
338+
// kpar_group .divide_group_comm(KPAR, false);
339+
// bndpar_group.divide_group_comm(BNDPAR, true);
340+
// -> sets POOL_WORLD, KP_WORLD, INT_BGROUP, BP_WORLD.
341+
Parallel_Global::divide_pools(world_nproc, my_rank, bndpar, kpar,
342+
GlobalV::NPROC_IN_BNDGROUP,
343+
GlobalV::RANK_IN_BPGROUP,
344+
GlobalV::MY_BNDGROUP,
345+
GlobalV::NPROC_IN_POOL,
346+
GlobalV::RANK_IN_POOL,
347+
GlobalV::MY_POOL);
348+
350349
return ParallelPartition(world_nproc,
351350
my_rank,
352351
kpar,
353-
my_pool_local,
354-
rank_in_pool_local,
352+
GlobalV::MY_POOL,
353+
GlobalV::RANK_IN_POOL,
355354
nproc_in_pool,
356355
bndpar,
357-
my_bndgroup,
358-
rank_in_bpgroup,
359-
nproc_in_bndgroup,
356+
GlobalV::MY_BNDGROUP,
357+
GlobalV::RANK_IN_BPGROUP,
358+
GlobalV::NPROC_IN_BNDGROUP,
360359
POOL_WORLD, // -> pw_world_comm (legacy duped handle)
361360
KP_WORLD, // -> kmesh_world_comm (KP_WORLD alias back)
362361
INT_BGROUP, // -> bsame_kdiff_world_comm

source/source_main/driver.cpp

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -151,38 +151,51 @@ void Driver::reading()
151151
// (*temp*) copy the variables from INPUT to each class
152152
Input_Conv::Convert();
153153

154-
// (4) define the 'DIAGONALIZATION' world in MPI
155-
Parallel_Global::split_diag_world(PARAM.inp.diago_proc,
156-
GlobalV::NPROC,
157-
GlobalV::MY_RANK,
158-
GlobalV::DRANK,
159-
GlobalV::DSIZE,
160-
GlobalV::DCOLOR);
161-
Parallel_Global::split_grid_world(PARAM.inp.diago_proc,
162-
GlobalV::NPROC,
163-
GlobalV::MY_RANK,
164-
GlobalV::GRANK,
165-
GlobalV::GSIZE);
154+
// (4)+(5) Build the full parallel partition in one factory call.
155+
// The factory internally reproduces the exact legacy call order:
156+
// 1. split_diag_world -> DIAG_WORLD
157+
// 2. split_grid_world -> GRID_WORLD
158+
// 3. divide_pools -> POOL_WORLD/KP_WORLD/INT_BGROUP/BP_WORLD
159+
// and writes the same GlobalV scalars via the legacy helpers.
160+
// split_diag_world / split_grid_world write DRANK/DSIZE/DCOLOR/
161+
// GRANK/GSIZE through their reference parameters; divide_pools
162+
// writes NPROC_IN_POOL/RANK_IN_POOL/MY_POOL/etc through
163+
// init_pools' reference parameters into GlobalV. We then read
164+
// those scalars back for the OUT prints, keeping behavior identical.
165+
this->topo_ = Parallel_Global::create_partition(GlobalV::NPROC,
166+
GlobalV::MY_RANK,
167+
GlobalV::KPAR,
168+
PARAM.inp.bndpar,
169+
PARAM.inp.diago_proc,
170+
PARAM.inp.diago_proc);
171+
172+
// The factory's split_diag_world / split_grid_world write to local
173+
// variables, not to GlobalV. Read the values back from the topology
174+
// handles so the GlobalV scalars and printed output match the old
175+
// driver exactly.
176+
GlobalV::DRANK = 0;
177+
GlobalV::DSIZE = 1;
178+
GlobalV::DCOLOR = 0;
179+
GlobalV::GRANK = 0;
180+
GlobalV::GSIZE = 1;
181+
#ifdef __MPI
182+
if (this->topo_.diag_world_comm() != MPI_COMM_NULL)
183+
{
184+
MPI_Comm_rank(this->topo_.diag_world_comm(), &GlobalV::DRANK);
185+
MPI_Comm_size(this->topo_.diag_world_comm(), &GlobalV::DSIZE);
186+
}
187+
if (this->topo_.rgrid_world_comm() != MPI_COMM_NULL)
188+
{
189+
MPI_Comm_rank(this->topo_.rgrid_world_comm(), &GlobalV::GRANK);
190+
MPI_Comm_size(this->topo_.rgrid_world_comm(), &GlobalV::GSIZE);
191+
GlobalV::DCOLOR = GlobalV::GRANK;
192+
}
193+
#endif
166194
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running, "DRANK", GlobalV::DRANK + 1);
167195
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running, "DSIZE", GlobalV::DSIZE);
168196
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running, "DCOLOR", GlobalV::DCOLOR + 1);
169197
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running, "GRANK", GlobalV::GRANK + 1);
170198
ModuleBase::GlobalFunc::OUT(GlobalV::ofs_running, "GSIZE", GlobalV::GSIZE);
171-
172-
#ifdef __MPI
173-
// (5) divide the GlobalV::NPROC processors into GlobalV::KPAR for k-points
174-
// parallelization.
175-
Parallel_Global::init_pools(GlobalV::NPROC,
176-
GlobalV::MY_RANK,
177-
PARAM.inp.bndpar,
178-
GlobalV::KPAR,
179-
GlobalV::NPROC_IN_BNDGROUP,
180-
GlobalV::RANK_IN_BPGROUP,
181-
GlobalV::MY_BNDGROUP,
182-
GlobalV::NPROC_IN_POOL,
183-
GlobalV::RANK_IN_POOL,
184-
GlobalV::MY_POOL);
185-
#endif
186199
ModuleBase::timer::end("Driver", "reading");
187200
}
188201

source/source_main/driver.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef DRIVER_H
22
#define DRIVER_H
33

4+
#include "source_base/parallel_partition.h"
45

56
class Driver
67
{
@@ -40,6 +41,10 @@ class Driver
4041
// Init harewares according to Input parameters
4142
void init_hardware();
4243
void finalize_hardware();
44+
45+
/// Parallel partition built once in reading() and injected into
46+
/// the ESolver in driver_run().
47+
ParallelPartition topo_;
4348
};
4449

4550
#endif

source/source_main/driver_run.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ void Driver::driver_run()
5959

6060
this->init_hardware();
6161
ModuleESolver::ESolver* p_esolver = ModuleESolver::init_esolver(PARAM.inp);
62+
p_esolver->set_topology(this->topo_);
6263

6364
// UnitCell is initialized only for workflows that require its full DFT state.
6465
UnitCell ucell;

0 commit comments

Comments
 (0)