Merge pull request #5857 from ye-luo/introduce-intra-group-comm

Replace group-leader Communicator with inter-group Communicator

Author: PDoakORNL

src/Message/Communicate.cpp

@@ -48,7 +48,7 @@ Communicate::~Communicate() = default;
 //exclusive:  MPI or Serial
 #ifdef HAVE_MPI
 
-  // in_comm needs to be mutable to be duplicated
+// in_comm needs to be mutable to be duplicated
 Communicate::Communicate(mpi3::communicator& in_comm) : d_groupid(0), d_ngroups(1), comm{in_comm.duplicate()}
 {
   myMPI       = comm.get();
@@ -66,7 +66,7 @@ Communicate::Communicate(mpi3::communicator&& in_comm) : d_groupid(0), d_ngroups
 Communicate::Communicate(const Communicate& in_comm, int nparts)
 {
   std::vector<int> nplist(nparts + 1);
-  int p = FairDivideLow(in_comm.rank(), in_comm.size(), nparts, nplist); //group
+  const int p = FairDivideLow(in_comm.rank(), in_comm.size(), nparts, nplist); //group
   // comm is mutable member
   comm  = in_comm.comm.split(p, in_comm.rank());
   myMPI = comm.get();
@@ -75,17 +75,13 @@ Communicate::Communicate(const Communicate& in_comm, int nparts)
   d_ncontexts = comm.size();
   d_groupid   = p;
   d_ngroups   = nparts;
-  // create a communicator among group leaders.
-
-  nplist.pop_back();
-  mpi3::communicator leader_comm = in_comm.comm.subcomm(nplist);
-  if (isGroupLeader())
-    GroupLeaderComm = std::make_unique<Communicate>(leader_comm);
-  else
-    GroupLeaderComm.reset();
+
+  // create an inter group communicator
+  inter_group_comm_ = std::make_unique<Communicate>(in_comm.comm.split(comm.rank(), in_comm.rank()));
 }
 
-Communicate Communicate::NodeComm() const {
+Communicate Communicate::NodeComm() const
+{
   // comm is mutable member
   return Communicate{comm.split_shared()};
 }
@@ -107,7 +103,7 @@ void Communicate::abort() const { comm.abort(1); }
 void Communicate::barrier() const { comm.barrier(); }
 #else
 
-Communicate Communicate::NodeComm() const {return Communicate{};}
+Communicate Communicate::NodeComm() const { return Communicate{}; }
 
 void Communicate::finalize() {}
 
@@ -119,9 +115,7 @@ void Communicate::cleanupMessage(void*) {}
 
 Communicate::Communicate(const Communicate& in_comm, int nparts)
     : myMPI(MPI_COMM_NULL), d_mycontext(0), d_ncontexts(1), d_groupid(0)
-{
-  GroupLeaderComm = std::make_unique<Communicate>();
-}
+{ inter_group_comm_ = std::make_unique<Communicate>(); }
 #endif // !HAVE_MPI
 
 void Communicate::barrier_and_abort(const std::string& msg) const

src/Message/Communicate.h

@@ -215,12 +215,12 @@ class Communicate : public CommunicatorTraits
   int d_groupid;
   /// Total number of groups in the parent communicator
   int d_ngroups;
-  /// Group Leader Communicator
-  std::unique_ptr<Communicate> GroupLeaderComm;
+  /// inter group communicator
+  std::unique_ptr<Communicate> inter_group_comm_;
 
 public:
   // Avoid public access to unique_ptr.
-  Communicate* getGroupLeaderComm() { return GroupLeaderComm.get(); }
+  Communicate& getInterGroupComm() const { return *inter_group_comm_; }
 
 #ifdef HAVE_MPI
   /// mpi3 communicator wrapper

src/Message/tests/test_communciate.cpp

@@ -24,19 +24,11 @@ TEST_CASE("test_communicate_split_one", "[message]")
   REQUIRE(c2->size() == c->size());
   REQUIRE(c2->rank() == c->rank());
 
-  if (c->rank() == 0)
-  {
-    REQUIRE(c2->isGroupLeader() == true);
-    auto GroupLeaderComm = c2->getGroupLeaderComm();
-    REQUIRE(GroupLeaderComm != nullptr);
-    REQUIRE(GroupLeaderComm->size() == 1);
-    REQUIRE(GroupLeaderComm->rank() == 0);
-  }
-  else
-  {
-    REQUIRE(c2->isGroupLeader() == false);
-    REQUIRE(c2->getGroupLeaderComm() == nullptr);
-  }
+  REQUIRE(c2->isGroupLeader() == (c->rank() == 0));
+
+  auto& inter_group_comm = c2->getInterGroupComm();
+  REQUIRE(inter_group_comm.size() == 1);
+  REQUIRE(inter_group_comm.rank() == 0);
 
   std::string real_name = c->getName();
 
@@ -66,37 +58,23 @@ TEST_CASE("test_communicate_split_two", "[message]")
     int new_group_id = c->rank() < midpoint ? 0 : 1;
     int new_rank     = c->rank();
     if (c->rank() >= midpoint)
-    {
       new_rank -= midpoint;
-    }
+
     // Adjust for odd size - the last group has the extra process
     if (c->size() % 2 == 1)
-    {
       new_size[1] = new_size[1] + 1;
-    }
+
     REQUIRE(c2->size() == new_size[new_group_id]);
     REQUIRE(c2->rank() == new_rank);
 
-    if (c->rank() == 0 || c->rank() == midpoint)
-    {
-      REQUIRE(c2->isGroupLeader() == true);
-      auto GroupLeaderComm = c2->getGroupLeaderComm();
-      REQUIRE(GroupLeaderComm != nullptr);
-      REQUIRE(GroupLeaderComm->size() == 2);
-      if (c->rank() == 0)
-      {
-        REQUIRE(GroupLeaderComm->rank() == 0);
-      }
-      else
-      {
-        REQUIRE(GroupLeaderComm->rank() == 1);
-      }
-    }
+    REQUIRE(c2->isGroupLeader() == (c->rank() == 0 || c->rank() == midpoint));
+
+    auto& inter_group_comm = c2->getInterGroupComm();
+    REQUIRE(inter_group_comm.size() == (c->rank() < midpoint * 2 ? 2 : 1));
+    if (c->rank() < midpoint * 2)
+      REQUIRE(inter_group_comm.rank() == new_group_id);
     else
-    {
-      REQUIRE(c2->isGroupLeader() == false);
-      REQUIRE(c2->getGroupLeaderComm() == nullptr);
-    }
+      REQUIRE(inter_group_comm.rank() == 0);
   }
 }
 
@@ -108,24 +86,14 @@ TEST_CASE("test_communicate_split_four", "[message]")
   {
     auto c2 = std::make_unique<Communicate>(*c, 4);
 
-    REQUIRE(c2->size() == c->size() / 4);
-    int group_size = c->size() / 4;
-    int new_rank   = c->rank() % group_size;
+    const int group_size = c->size() / 4;
+    const int new_rank   = c->rank() % group_size;
+    REQUIRE(c2->size() == group_size);
     REQUIRE(c2->rank() == new_rank);
-
-    if (new_rank == 0)
-    {
-      REQUIRE(c2->isGroupLeader() == true);
-      auto GroupLeaderComm = c2->getGroupLeaderComm();
-      REQUIRE(GroupLeaderComm != nullptr);
-      REQUIRE(GroupLeaderComm->size() == 4);
-      REQUIRE(GroupLeaderComm->rank() == c->rank() / group_size);
-    }
-    else
-    {
-      REQUIRE(c2->isGroupLeader() == false);
-      REQUIRE(c2->getGroupLeaderComm() == nullptr);
-    }
+    REQUIRE(c2->isGroupLeader() == (new_rank == 0));
+    auto& inter_group_comm = c2->getInterGroupComm();
+    REQUIRE(inter_group_comm.size() == 4);
+    REQUIRE(inter_group_comm.rank() == c->rank() / group_size);
   }
 }
 

src/QMCWaveFunctions/BsplineFactory/HybridRepCenterOrbitals.cpp

@@ -51,19 +51,19 @@ bool AtomicOrbitals<ST>::write_splines(hdf_archive& h5f)
 }
 
 template<typename ST>
-void HybridRepCenterOrbitals<ST>::bcast_atomic_tables(Communicate* comm)
+void HybridRepCenterOrbitals<ST>::bcast_atomic_tables(Communicate& comm)
 {
   for (int ic = 0; ic < AtomicCenters.size(); ic++)
-    AtomicCenters[ic].bcast_tables(*comm);
+    AtomicCenters[ic].bcast_tables(comm);
 }
 
 template<typename ST>
-void HybridRepCenterOrbitals<ST>::gather_atomic_tables(Communicate* comm, const std::vector<int>& offset)
+void HybridRepCenterOrbitals<ST>::gather_atomic_tables(Communicate& comm, const std::vector<int>& offset)
 {
-  if (comm->size() == 1)
+  if (comm.size() == 1)
     return;
   for (int ic = 0; ic < AtomicCenters.size(); ic++)
-    AtomicCenters[ic].gather_tables(*comm, offset);
+    AtomicCenters[ic].gather_tables(comm, offset);
 }
 
 template<typename ST>

src/QMCWaveFunctions/BsplineFactory/HybridRepCenterOrbitals.h

@@ -550,8 +550,8 @@ class HybridRepCenterOrbitals
               << "for the atomic radial splines in hybrid orbital representation" << std::endl;
   }
 
-  void bcast_atomic_tables(Communicate* comm);
-  void gather_atomic_tables(Communicate* comm, const std::vector<int>& offset);
+  void bcast_atomic_tables(Communicate& comm);
+  void gather_atomic_tables(Communicate& comm, const std::vector<int>& offset);
 
   inline void flush_zero()
   {

src/QMCWaveFunctions/BsplineFactory/HybridRepSetReader.cpp

@@ -280,7 +280,7 @@ std::unique_ptr<SPOSet> HybridRepSetReader<ST>::create_spline_set(const std::str
   {
     Timer now;
     SplineUtils<ST>::bcast(multi_splines, *myComm);
-    hybrid_center_orbs.bcast_atomic_tables(myComm);
+    hybrid_center_orbs.bcast_atomic_tables(*myComm);
     app_log() << "  Time to bcast the table = " << now.elapsed() << std::endl;
   }
   return bspline;
@@ -751,19 +751,20 @@ void HybridRepSetReader<ST>::initialize_hybrid_pio_gather(const int spin,
   myComm->barrier();
   if (band_group_comm.isGroupLeader())
   {
+    auto& group_leader_comm = band_group_comm.getInterGroupComm();
     Timer now;
     if (use_duplex_splines_)
     {
       std::vector<int> offset(band_groups.size());
       for (int i = 0; i < offset.size(); i++)
         offset[i] = band_groups[i] * 2;
-      SplineUtils<ST>::gatherv(multi_splines, offset, *band_group_comm.getGroupLeaderComm());
-      multi_atomic_splines.gather_atomic_tables(band_group_comm.getGroupLeaderComm(), offset);
+      SplineUtils<ST>::gatherv(multi_splines, offset, group_leader_comm);
+      multi_atomic_splines.gather_atomic_tables(group_leader_comm, offset);
     }
     else
     {
-      SplineUtils<ST>::gatherv(multi_splines, band_groups, *band_group_comm.getGroupLeaderComm());
-      multi_atomic_splines.gather_atomic_tables(band_group_comm.getGroupLeaderComm(), band_groups);
+      SplineUtils<ST>::gatherv(multi_splines, band_groups, group_leader_comm);
+      multi_atomic_splines.gather_atomic_tables(group_leader_comm, band_groups);
     }
 
     app_log() << "  Time to gather the table = " << now.elapsed() << std::endl;

src/QMCWaveFunctions/BsplineFactory/SplineSetReader.cpp

@@ -176,6 +176,7 @@ void SplineSetReader<ST>::initialize_spline_pio_gather(const int spin,
   const std::vector<BandInfo>& cur_bands = bandgroup.myBands;
   if (band_group_comm.isGroupLeader())
   {
+    auto& group_leader_comm = band_group_comm.getInterGroupComm();
     h5f.open(mybuilder->H5FileName, H5F_ACC_RDONLY);
     for (int iorb = iorb_first; iorb < iorb_last; iorb++)
     {
@@ -193,17 +194,17 @@ void SplineSetReader<ST>::initialize_spline_pio_gather(const int spin,
     }
 
     {
-      band_group_comm.getGroupLeaderComm()->barrier();
+      group_leader_comm.barrier();
       Timer now;
       if (use_duplex_splines_)
       {
         std::vector<int> offset(band_groups.size());
         for (int i = 0; i < offset.size(); i++)
           offset[i] = band_groups[i] * 2;
-        SplineUtils<ST>::gatherv(multi_splines, offset, *band_group_comm.getGroupLeaderComm());
+        SplineUtils<ST>::gatherv(multi_splines, offset, group_leader_comm);
       }
       else
-        SplineUtils<ST>::gatherv(multi_splines, band_groups, *band_group_comm.getGroupLeaderComm());
+        SplineUtils<ST>::gatherv(multi_splines, band_groups, group_leader_comm);
       app_log() << "  Time to gather the table = " << now.elapsed() << std::endl;
     }
   }