Skip to content

Commit d185d2e

Browse files
author
abacus_fixer
committed
Replace Parallel_Reduce with ParaKmeshWorld in occupy, fix ParaWorld ODR
Migrate occupy.cpp and elecstate_tools.cpp from the legacy Parallel_Reduce::reduce_double_allpool / reduce_max / reduce_min to the new module_parallel ParaKmeshWorld API: - Add reduce_across_pools / reduce_max_across_pools / reduce_min_across_pools to ParaKmeshWorld (Allreduce on the inter-pool communicator KP_WORLD, no-op when kpar==1). - Add a reduce-only ParaKmeshWorld() default constructor and a no-arg make_kmesh_world() bridge overload for call sites like calEBand that have no k-point information to pass. - occupy.h/cpp: replace npool parameter with const ParaKmeshWorld&, remove #include parallel_reduce.h, remove GlobalV::NPROC_IN_POOL. - elecstate_tools.cpp: calEBand and calculate_weights now construct kmesh via the bridge; delete the old npool/bndpar local variable. Fix a latent ODR violation in ParaWorld that caused a free() crash in test_occupy: the MPI_Comm comm_ member only existed under #ifdef __MPI, so the class layout differed between translation units compiled with and without __MPI. Replace with a void* opaque handle (memcpy round-trip, layout-identical in both builds).
1 parent 792c1c5 commit d185d2e

10 files changed

Lines changed: 242 additions & 65 deletions

File tree

source/source_base/module_parallel/para_bridge.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include "para_tag.h"
33

44
#ifdef __MPI
5+
#include "source_base/global_variable.h"
56
#include "source_base/parallel_comm.h"
67
#endif
78

@@ -19,4 +20,40 @@ ParaWorld make_pw_world()
1920
#endif
2021
}
2122

23+
// Reduce-only overload: no k-point distribution data.
24+
ParaKmeshWorld make_kmesh_world()
25+
{
26+
#ifdef __MPI
27+
int mpi_initialized = 0;
28+
MPI_Initialized(&mpi_initialized);
29+
if (mpi_initialized && GlobalV::KPAR > 1 && KP_WORLD != MPI_COMM_NULL)
30+
{
31+
// Build from globals but skip distribute_kpoints (nkstot=0).
32+
return ParaKmeshWorld(KP_WORLD, GlobalV::KPAR, GlobalV::MY_POOL,
33+
GlobalV::NPROC, 0, 1);
34+
}
35+
#endif
36+
return ParaKmeshWorld();
37+
}
38+
39+
// Temporary bridge: construct a kmesh-domain ParaKmeshWorld from the old
40+
// globals. Delete this file once ParaCollection is wired into driver init.
41+
ParaKmeshWorld make_kmesh_world(int nkstot, int nspin)
42+
{
43+
#ifdef __MPI
44+
// Fall back to a serial single-pool domain when MPI is not initialized
45+
// (e.g. unit tests linked against the MPI-compiled base library) or
46+
// when there is only one k-point pool, so that no MPI call is made on
47+
// an unset communicator.
48+
int mpi_initialized = 0;
49+
MPI_Initialized(&mpi_initialized);
50+
if (mpi_initialized && GlobalV::KPAR > 1 && KP_WORLD != MPI_COMM_NULL)
51+
{
52+
return ParaKmeshWorld(KP_WORLD, GlobalV::KPAR, GlobalV::MY_POOL,
53+
GlobalV::NPROC, nkstot, nspin);
54+
}
55+
#endif
56+
return ParaKmeshWorld(nkstot, nspin);
57+
}
58+
2259
} // namespace Parallel

source/source_base/module_parallel/para_bridge.h

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

4+
#include "para_kmesh_world.h"
45
#include "para_world.h"
56

67
namespace Parallel
@@ -16,6 +17,32 @@ namespace Parallel
1617
*/
1718
ParaWorld make_pw_world();
1819

20+
/**
21+
* @brief Temporary bridge: construct a kmesh-domain ParaKmeshWorld from
22+
* the old globals KP_WORLD / GlobalV::KPAR (MPI) or as a serial domain
23+
* (non-MPI).
24+
*
25+
* Falls back to a serial single-pool domain when MPI is not initialized
26+
* (e.g. unit tests linked against the MPI-compiled base library) or when
27+
* there is only one k-point pool, so that no MPI call is made on an
28+
* unset communicator.
29+
*
30+
* @param[in] nkstot total number of k-points (without spin)
31+
* @param[in] nspin number of spin components
32+
*/
33+
ParaKmeshWorld make_kmesh_world(int nkstot, int nspin);
34+
35+
/**
36+
* @brief Reduce-only overload: construct a kmesh domain for call sites
37+
* that only need cross-pool reduction (reduce_across_pools etc.) and
38+
* have no k-point information to pass.
39+
*
40+
* The k-point distribution data (nks_pool_, whichpool_, ...) is left
41+
* empty; calling pool_collection / gather_kvec on the returned object
42+
* is invalid. Use the (nkstot, nspin) overload when those are needed.
43+
*/
44+
ParaKmeshWorld make_kmesh_world();
45+
1946
} // namespace Parallel
2047

2148
#endif // PARA_BRIDGE_H

source/source_base/module_parallel/para_kmesh_world.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ ParaKmeshWorld::ParaKmeshWorld(int nkstot, int nspin)
1515
startk_global_ = 0;
1616
}
1717

18+
ParaKmeshWorld::ParaKmeshWorld()
19+
: ParaWorld("kmesh"), kpar_(1), my_pool_(0), rank_in_pool_(0),
20+
nproc_(1), nspin_(1), nkstot_(0), nks_local_(0), startk_global_(0)
21+
{
22+
// Intentionally empty: no k-point distribution data.
23+
// Only kpar_ / comm() are valid for reduce_across_pools.
24+
}
25+
1826
#ifdef __MPI
1927
ParaKmeshWorld::ParaKmeshWorld(const MPI_Comm& comm, int kpar, int my_pool, int nproc, int nkstot, int nspin)
2028
: ParaWorld("kmesh", comm), kpar_(kpar), my_pool_(my_pool),
@@ -93,6 +101,39 @@ int ParaKmeshWorld::max_nks_pool() const
93101
return *std::max_element(nks_pool_.begin(), nks_pool_.end());
94102
}
95103

104+
void ParaKmeshWorld::reduce_across_pools(double& value) const
105+
{
106+
if (kpar_ == 1)
107+
{
108+
return;
109+
}
110+
#ifdef __MPI
111+
MPI_Allreduce(MPI_IN_PLACE, &value, 1, MPI_DOUBLE, MPI_SUM, comm());
112+
#endif
113+
}
114+
115+
void ParaKmeshWorld::reduce_max_across_pools(double& value) const
116+
{
117+
if (kpar_ == 1)
118+
{
119+
return;
120+
}
121+
#ifdef __MPI
122+
MPI_Allreduce(MPI_IN_PLACE, &value, 1, MPI_DOUBLE, MPI_MAX, comm());
123+
#endif
124+
}
125+
126+
void ParaKmeshWorld::reduce_min_across_pools(double& value) const
127+
{
128+
if (kpar_ == 1)
129+
{
130+
return;
131+
}
132+
#ifdef __MPI
133+
MPI_Allreduce(MPI_IN_PLACE, &value, 1, MPI_DOUBLE, MPI_MIN, comm());
134+
#endif
135+
}
136+
96137
void ParaKmeshWorld::pool_collection(double& value, const double* wk, int ik) const
97138
{
98139
#ifdef __MPI

source/source_base/module_parallel/para_kmesh_world.h

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ class ParaKmeshWorld : public ParaWorld
3030
*/
3131
ParaKmeshWorld(int nkstot, int nspin);
3232

33+
/**
34+
* @brief Construct a reduce-only k-mesh domain with no k-point
35+
* distribution data.
36+
*
37+
* kpar_/comm are set from the bridge globals so that
38+
* reduce_across_pools / reduce_max/min_across_pools work correctly.
39+
* The distribution arrays (nks_pool_, whichpool_, ...) are left
40+
* empty; calling pool_collection / gather_kvec is invalid.
41+
*/
42+
ParaKmeshWorld();
43+
3344
#ifdef __MPI
3445
/**
3546
* @brief Construct a k-mesh domain on an existing communicator.
@@ -83,6 +94,36 @@ class ParaKmeshWorld : public ParaWorld
8394
/// Maximum number of k-points across all pools.
8495
int max_nks_pool() const;
8596

97+
// ===== Cross-pool reductions =====
98+
99+
/**
100+
* @brief Sum a scalar across all k-point pools.
101+
*
102+
* Replaces Parallel_Reduce::reduce_double_allpool. Uses the inter-pool
103+
* communicator (comm()) so that same-rank processes across pools
104+
* participate. Since all processes in a pool share the same value,
105+
* no normalization by pool size is needed. No-op when kpar()==1.
106+
*
107+
* @param[in,out] value local partial sum, overwritten with global total
108+
*/
109+
void reduce_across_pools(double& value) const;
110+
111+
/**
112+
* @brief Global max across all k-point pools.
113+
*
114+
* @param[in,out] value local value, overwritten with global max
115+
*/
116+
void reduce_max_across_pools(double& value) const;
117+
118+
/**
119+
* @brief Global min across all k-point pools.
120+
*
121+
* @param[in,out] value local value, overwritten with global min
122+
*/
123+
void reduce_min_across_pools(double& value) const;
124+
125+
// ===== Cross-domain operations =====
126+
86127
/**
87128
* @brief Collect a scalar value from the pool that owns k-point ik.
88129
*

source/source_base/module_parallel/para_world.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,23 @@
33
namespace Parallel
44
{
55

6-
ParaWorld::ParaWorld(const std::string& tag) : tag_(tag), rank_(0), size_(1)
6+
ParaWorld::ParaWorld(const std::string& tag) : tag_(tag), rank_(0), size_(1), comm_(nullptr)
77
{
88
#ifdef __MPI
99
if (!tag.empty())
1010
{
11-
comm_ = MPI_COMM_SELF;
11+
comm_ = handle_from_comm(MPI_COMM_SELF);
1212
}
1313
else
1414
{
15-
comm_ = MPI_COMM_NULL;
15+
comm_ = handle_from_comm(MPI_COMM_NULL);
1616
}
1717
#endif
1818
}
1919

2020
#ifdef __MPI
21-
ParaWorld::ParaWorld(const std::string& tag, const MPI_Comm& comm) : tag_(tag), comm_(comm)
21+
ParaWorld::ParaWorld(const std::string& tag, const MPI_Comm& comm)
22+
: tag_(tag), comm_(handle_from_comm(comm))
2223
{
2324
if (comm == MPI_COMM_NULL)
2425
{
@@ -34,7 +35,7 @@ ParaWorld::ParaWorld(const std::string& tag, const MPI_Comm& comm) : tag_(tag),
3435
bool ParaWorld::valid() const
3536
{
3637
#ifdef __MPI
37-
return comm_ != MPI_COMM_NULL;
38+
return comm() != MPI_COMM_NULL;
3839
#else
3940
return !tag_.empty();
4041
#endif

source/source_base/module_parallel/para_world.h

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

4+
#include <cstring>
45
#include <memory>
56
#include <string>
67

@@ -20,9 +21,13 @@ namespace Parallel
2021
* GlobalV::RANK_IN_POOL / POOL_WORLD by an object that functions
2122
* receive explicitly.
2223
*
23-
* In serial builds (no __MPI) the communicator member does not
24-
* exist; rank() always returns 0 and size() always returns 1, so
25-
* call sites compile unchanged in both serial and MPI builds.
24+
* The communicator is stored as an opaque handle so that the class
25+
* layout is identical in serial and MPI builds. Binaries that mix
26+
* translation units compiled with different __MPI settings (e.g. unit
27+
* tests linked against the MPI-compiled base library) would otherwise
28+
* be an ODR violation with undefined behavior. In serial builds rank()
29+
* always returns 0 and size() always returns 1, so call sites compile
30+
* unchanged in both serial and MPI builds.
2631
*/
2732
class ParaWorld
2833
{
@@ -59,7 +64,11 @@ class ParaWorld
5964
/// Underlying MPI communicator (MPI builds only).
6065
MPI_Comm comm() const
6166
{
62-
return comm_;
67+
MPI_Comm comm = MPI_COMM_NULL;
68+
static_assert(sizeof(MPI_Comm) <= sizeof(comm_),
69+
"MPI_Comm does not fit into the opaque handle");
70+
std::memcpy(&comm, &comm_, sizeof(MPI_Comm));
71+
return comm;
6372
}
6473
#endif
6574

@@ -127,12 +136,23 @@ class ParaWorld
127136
#endif
128137

129138
private:
139+
#ifdef __MPI
140+
/// Wrap an MPI communicator into the opaque handle storage.
141+
static void* handle_from_comm(const MPI_Comm& comm)
142+
{
143+
void* handle = nullptr;
144+
std::memcpy(&handle, &comm, sizeof(MPI_Comm));
145+
return handle;
146+
}
147+
#endif
148+
130149
std::string tag_; ///< domain tag
131150
int rank_; ///< rank inside domain
132151
int size_; ///< number of processes in domain
133-
#ifdef __MPI
134-
MPI_Comm comm_; ///< wrapped communicator (never owned/freed here)
135-
#endif
152+
// Opaque communicator handle, present in both serial and MPI builds
153+
// so that the class layout never depends on the __MPI macro (see the
154+
// class comment). Never owned/freed here.
155+
void* comm_;
136156
};
137157

138158
} // namespace Parallel

0 commit comments

Comments
 (0)