remove PCU original comm and ownership and use Dup

- pcu/PCU.h: remove GetMPIComm, GetComm, SwitchMPIComm, SwitchComm,
  OwnsComm, OwnsComm(bool).
- (PCU::Split): change return type to int and set output pointer.
- update documentation.
- pcu/PCU.cc: remove GetMPIComm, GetComm, SwitchMPIComm, SwitchComm,
  OwnsComm.
- (PCU::Split): remove OwnsComm call.
- (PCU::DupComm): return int and set pointer.
- pcu/pcu_mpi.h: remove pcu_mpi_struct original_comm and owned.
- pcu/pcu_pmpi.c (pcu_pmpi_init): do not copy original comm and remove
  self->owned.
- (pcu_pmpi_finalize): remove MPI_Comm_free for owned original_comm.
- (pcu_pmpi_split): split over user_comm.
- pcu/pcu_pnompi.c: update init and finalize.
- pcu/PCU_C.h: remove Switch_Comm and Get_Comm.
- add Comm_Dup and Comm_Split.
- pcu/pcu_c.cc: remove Switch_Comm and Get_Comm. add Comm_Dup with
  pointer to newpcu that releases the unique_ptr and Comm_Split.

Signed-off-by: Aiden Woodruff <[email protected]>

pcu/PCU.cc

@@ -148,13 +148,10 @@ std::unique_ptr<PCU> PCU::Split(int color, int key) noexcept {
   PCU_Comm newcomm;
   pcu_mpi_split(mpi_, color, key, &newcomm);
   PCU* splitpcu = new PCU(newcomm);
-  splitpcu->OwnsComm(true);
   return std::unique_ptr<PCU>(splitpcu);
 }
-PCU_Comm PCU::DupComm() const noexcept {
-  PCU_Comm newcomm;
-  pcu_mpi_dup(mpi_, &newcomm);
-  return newcomm;
+int PCU::DupComm(PCU_Comm* newcomm) const noexcept {
+  return pcu_mpi_dup(mpi_, newcomm);
 }
 
 int PCU::Packed(int to_rank, size_t *size) noexcept {
@@ -280,23 +277,6 @@ PCU &PCU::operator=(PCU && other) noexcept {
   std::swap(msg_, other.msg_);
   return *this;
 }
-PCU_Comm PCU::GetMPIComm() const noexcept { return GetComm(); }
-PCU_Comm PCU::GetComm() const noexcept { return mpi_->original_comm; }
-bool PCU::OwnsComm() const noexcept { return mpi_->owned; }
-void PCU::OwnsComm(bool on) const noexcept { mpi_->owned = on ? 1 : 0; }
-
-PCU_Comm PCU::SwitchMPIComm(PCU_Comm newcomm) noexcept {
-  return SwitchComm(newcomm);
-}
-PCU_Comm PCU::SwitchComm(PCU_Comm newcomm) noexcept {
-  if(newcomm == mpi_->original_comm) {
-    return mpi_->original_comm;
-  }
-  auto original_comm = mpi_->original_comm;
-  pcu_mpi_finalize(mpi_);
-  pcu_mpi_init(newcomm, mpi_);
-  return original_comm;
-}
 
 /* template implementations */
 template <typename T> void PCU::Add(T *p, size_t n) noexcept {

pcu/PCU.h

@@ -27,23 +27,6 @@ class PCU {
    *  @return The number of ranks in the communicator.
    */
   [[nodiscard]] int Peers() const noexcept;
-  [[deprecated("Use PCU::GetComm instead.")]]
-  [[nodiscard]] PCU_Comm GetMPIComm() const noexcept;
-  /**
-   * @brief Get the underlying communicator which may be an MPI_Comm.
-   */
-  [[nodiscard]] PCU_Comm GetComm() const noexcept;
-  /** @brief Check if the original PCU_Comm is owned by this object.
-   * If true, it will be freed during destruction. */
-  bool OwnsComm() const noexcept;
-  /** @brief Set ownership of the orignal PCU_Comm passed to the constructor.
-   *
-   * This function can enable or disable ownership. If a communicator created
-   * with PCU::Split() is disowned, it should be freed by the user.
-   *
-   * @param on true to enable ownership.
-   */
-  void OwnsComm(bool on) const noexcept;
 
   [[nodiscard]] PCU_t GetCHandle() {PCU_t h; h.ptr=this; return h;}
   /*recommended message passing API*/
@@ -115,9 +98,10 @@ class PCU {
    * If SCOREC::core was compiled with the SCOREC_NO_MPI flag, the return value
    * is not meaningful.
    *
-   * @return The new duplicated communicator.
+   * @param[out] newcomm The output address for the new communicator copy.
+   * @return 0 on success.
    */
-  PCU_Comm DupComm() const noexcept;
+  int DupComm(PCU_Comm* newcomm) const noexcept;
 
   /*lesser-used APIs*/
   int Packed(int to_rank, size_t *size) noexcept;
@@ -130,17 +114,6 @@ class PCU {
   /* Debug functions */
   void DebugOpen() noexcept;
 
-  [[deprecated("Use PCU::SwitchComm instead.")]]
-#if __cplusplus >= 201703L
-  [[nodiscard]]
-#endif
-  PCU_Comm SwitchMPIComm(PCU_Comm) noexcept;
-
-#if __cplusplus >= 201703L
-  [[nodiscard]]
-#endif
-  PCU_Comm SwitchComm(PCU_Comm) noexcept;
-
 private:
   pcu_msg_struct *msg_;
   pcu_mpi_struct *mpi_;

pcu/PCU_C.h

@@ -21,6 +21,9 @@ int PCU_Comm_Free(PCU_t* h);
 int PCU_Comm_Self(PCU_t h);
 int PCU_Comm_Peers(PCU_t h);
 
+int PCU_Comm_Dup(PCU_t h, PCU_Comm* newcomm);
+void PCU_Comm_Split(PCU_t h, int color, int key, PCU_t* newpcu);
+
 /*recommended message passing API*/
 void PCU_Comm_Begin(PCU_t h);
 int PCU_Comm_Pack(PCU_t h, int to_rank, const void* data, size_t size);
@@ -95,10 +98,6 @@ int PCU_Comm_Size(PCU_t h, int* size);
 
 /*deprecated method enum*/
 
-/*special MPI_Comm replacement API*/
-void PCU_Switch_Comm(PCU_t h, PCU_Comm new_comm);
-PCU_Comm PCU_Get_Comm(PCU_t h);
-
 /*stack trace helpers using GNU/Linux*/
 void PCU_Protect(void);
 

pcu/pcu_c.cc

@@ -36,6 +36,23 @@ int PCU_Comm_Peers(PCU_t h) {
   return static_cast<pcu::PCU*>(h.ptr)->Peers();
 }
 
+int PCU_Comm_Dup(PCU_t h, PCU_Comm* newcomm) {
+  if (h.ptr == nullptr)
+    reel_fail("PCU_Comm_Dup called before PCU_Comm_Init");
+  return static_cast<pcu::PCU*>(h.ptr)->DupComm(newcomm);
+}
+
+void PCU_Comm_Split(PCU_t h, int color, int key, PCU_t* newpcu) {
+  if (h.ptr == nullptr)
+    reel_fail("PCU_Comm_Split called before PCU_Comm_Init");
+  if (newpcu == nullptr)
+    reel_fail("PCU_Comm_Split received NULL newpcu.");
+  if (newpcu->ptr != nullptr)
+    reel_fail("PCU_Comm_Split received an initialized newpcu.");
+  auto newpcu_ptr = static_cast<pcu::PCU*>(h.ptr)->Split(color, key);
+  newpcu->ptr = newpcu_ptr.release();
+}
+
 void PCU_Comm_Begin(PCU_t h) {
   if (h.ptr == nullptr)
     reel_fail("Comm_Begin called before Comm_Init");
@@ -369,7 +386,6 @@ int PCU_Comm_Size(PCU_t h, int *size) {
 bool PCU_Comm_Initialized(PCU_t h) { return h.ptr != nullptr; }
 
 
-
 /** \brief Returns in * \a size the number of bytes being sent to \a to_rank.
   \details Returns the size of the buffer being sent to \a to_rank.
   This function should be called after PCU_Comm_Start and before
@@ -467,30 +483,6 @@ void *PCU_Comm_Extract(PCU_t h, size_t size) {
   return static_cast<pcu::PCU*>(h.ptr)->Extract(size);
 }
 
-/** \brief Reinitializes PCU with a new MPI communicator.
- \details All of PCU's logic is based off two duplicates
- of this communicator, so you can safely get PCU to act
- on sub-groups of processes using this function.
- This call should be collective over all processes
- in the previous communicator. This is a very heavy weight function
- and should be used sparingly.
- */
-void PCU_Switch_Comm(PCU_t h, PCU_Comm new_comm) {
-  if (h.ptr == nullptr)
-    reel_fail("Switch_Comm called before Comm_Init");
-  static_cast<pcu::PCU*>(h.ptr)->SwitchComm(new_comm);
-}
-
-/** \brief Return the current MPI communicator
-  \details Returns the communicator given to the
-  most recent PCU_Switch_Comm call, or MPI_COMM_WORLD
-  otherwise.
- */
-PCU_Comm PCU_Get_Comm(PCU_t h) {
-  if (h.ptr == nullptr)
-    reel_fail("Get_Comm called before Comm_Init");
-  return static_cast<pcu::PCU*>(h.ptr)->GetComm();
-}
 
 /** \brief Return the time in seconds since some time in the past
  */

pcu/pcu_mpi.h

@@ -29,12 +29,10 @@ void pcu_free_message(pcu_message* m);
 
 struct pcu_mpi_struct
 {
-  PCU_Comm original_comm;
   PCU_Comm user_comm;
   PCU_Comm coll_comm;
   int rank;
   int size;
-  int owned;
 };
 typedef struct pcu_mpi_struct pcu_mpi_t;
 

pcu/pcu_pmpi.c

@@ -18,28 +18,21 @@ bool pcu_pmpi_receive2(const pcu_mpi_t*, pcu_message* m, int tag, MPI_Comm comm)
 
 void pcu_pmpi_init(MPI_Comm comm, pcu_mpi_t* self)
 {
-  self->original_comm = comm;
   MPI_Comm_dup(comm,&(self->user_comm));
   MPI_Comm_dup(comm,&(self->coll_comm));
   MPI_Comm_size(comm,&(self->size));
   MPI_Comm_rank(comm,&(self->rank));
-  self->owned = 0;
 }
 
 void pcu_pmpi_finalize(pcu_mpi_t* self)
 {
   MPI_Comm_free(&(self->user_comm));
   MPI_Comm_free(&(self->coll_comm));
-  // Prevent accidental freeing of MPI_COMM_WORLD.
-  int result;
-  MPI_Comm_compare(self->original_comm, MPI_COMM_WORLD, &result);
-  if (self->owned && result != MPI_IDENT)
-    MPI_Comm_free(&(self->original_comm));
 }
 
 int pcu_pmpi_split(const pcu_mpi_t *mpi, int color, int key, MPI_Comm* newcomm)
 {
-  return MPI_Comm_split(mpi->original_comm,color,key,newcomm);
+  return MPI_Comm_split(mpi->user_comm,color,key,newcomm);
 }
 
 int pcu_pmpi_dup(const pcu_mpi_t *mpi, PCU_Comm* newcomm)

pcu/pcu_pnompi.c

@@ -104,12 +104,10 @@ void free_nompi_msg(NoMpiMsg* msg)
 //
 
 void pcu_pmpi_init(PCU_Comm comm, pcu_mpi_t *self) {
-  self->original_comm = comm;
   self->user_comm = comm+1;
   self->coll_comm = comm+2;
   self->size = 1;
   self->rank = 0;
-  self->owned = 0;
 }
 
 void pcu_pmpi_finalize(pcu_mpi_t* self) {