[coll-comm] review update:

- use consistent naming scheme
- fix include guards
- update docs

Co-authored-by: Pratik Nayak <pratik.nayak@kit.edu>

core/distributed/device_partition.hpp

@@ -2,8 +2,8 @@
 //
 // SPDX-License-Identifier: BSD-3-Clause
 
-#ifndef GINKGO_PARTITION_HPP
-#define GINKGO_PARTITION_HPP
+#ifndef GKO_CORE_DISTRIBUTED_PARTITION_HPP
+#define GKO_CORE_DISTRIBUTED_PARTITION_HPP
 
 #include <ginkgo/core/distributed/partition.hpp>
 
@@ -89,4 +89,4 @@ to_device_const(
 } // namespace gko
 
 
-#endif // GINKGO_PARTITION_HPP
+#endif // GKO_CORE_DISTRIBUTED_PARTITION_HPP

core/distributed/neighborhood_communicator.cpp

@@ -84,8 +84,9 @@ mpi::communicator create_neighborhood_comm(
  base.force_host_buffer());
 }
 
-std::unique_ptr<collective_communicator>
-neighborhood_communicator::create_inverse() const
+
+std::unique_ptr<CollectiveCommunicator>
+NeighborhoodCommunicator::create_inverse() const
 {
  auto base_comm = this->get_base_communicator();
  distributed::comm_index_type num_sources;
@@ -100,33 +101,32 @@ neighborhood_communicator::create_inverse() const
  comm_.get(), num_sources, sources.data(), MPI_UNWEIGHTED,
  num_destinations, destinations.data(), MPI_UNWEIGHTED));
 
- return std::unique_ptr<collective_communicator>{
- new neighborhood_communicator(base_comm, destinations, recv_sizes_,
- recv_offsets_, sources, send_sizes_,
- send_offsets_)};
+ return std::unique_ptr<CollectiveCommunicator>{new NeighborhoodCommunicator(
+ base_comm, destinations, recv_sizes_, recv_offsets_, sources,
+ send_sizes_, send_offsets_)};
 }
 
 
-comm_index_type neighborhood_communicator::get_recv_size() const
+comm_index_type NeighborhoodCommunicator::get_recv_size() const
 {
  return recv_offsets_.back();
 }
 
 
-comm_index_type neighborhood_communicator::get_send_size() const
+comm_index_type NeighborhoodCommunicator::get_send_size() const
 {
  return send_offsets_.back();
 }
 
 
-neighborhood_communicator::neighborhood_communicator(
+NeighborhoodCommunicator::NeighborhoodCommunicator(
  communicator base, const std::vector<distributed::comm_index_type>& sources,
  const std::vector<comm_index_type>& send_sizes,
  const std::vector<comm_index_type>& send_offsets,
  const std::vector<distributed::comm_index_type>& destinations,
  const std::vector<comm_index_type>& recv_sizes,
  const std::vector<comm_index_type>& recv_offsets)
- : collective_communicator(base), comm_(MPI_COMM_NULL)
+ : CollectiveCommunicator(base), comm_(MPI_COMM_NULL)
 {
  comm_ = create_neighborhood_comm(base, sources, destinations);
  send_sizes_ = send_sizes;
@@ -136,8 +136,8 @@ neighborhood_communicator::neighborhood_communicator(
 }
 
 
-neighborhood_communicator::neighborhood_communicator(communicator base)
- : collective_communicator(std::move(base)),
+NeighborhoodCommunicator::NeighborhoodCommunicator(communicator base)
+ : CollectiveCommunicator(std::move(base)),
  comm_(MPI_COMM_SELF),
  send_sizes_(),
  send_offsets_(1),
@@ -152,7 +152,7 @@ neighborhood_communicator::neighborhood_communicator(communicator base)
 }
 
 
-request neighborhood_communicator::i_all_to_all_v(
+request NeighborhoodCommunicator::i_all_to_all_v(
  std::shared_ptr<const Executor> exec, const void* send_buffer,
  MPI_Datatype send_type, void* recv_buffer, MPI_Datatype recv_type) const
 {
@@ -166,24 +166,24 @@ request neighborhood_communicator::i_all_to_all_v(
 }
 
 
-std::unique_ptr<collective_communicator>
-neighborhood_communicator::create_with_same_type(
+std::unique_ptr<CollectiveCommunicator>
+NeighborhoodCommunicator::create_with_same_type(
  communicator base, const distributed::index_map_variant& imap) const
 {
  return std::visit(
  [base](const auto& imap) {
- return std::unique_ptr<collective_communicator>(
- new neighborhood_communicator(base, imap));
+ return std::unique_ptr<CollectiveCommunicator>(
+ new NeighborhoodCommunicator(base, imap));
  },
  imap);
 }
 
 
 template <typename LocalIndexType, typename GlobalIndexType>
-neighborhood_communicator::neighborhood_communicator(
+NeighborhoodCommunicator::NeighborhoodCommunicator(
  communicator base,
  const distributed::index_map<LocalIndexType, GlobalIndexType>& imap)
- : collective_communicator(base),
+ : CollectiveCommunicator(base),
  comm_(MPI_COMM_SELF),
  recv_sizes_(imap.get_remote_target_ids().get_size()),
  recv_offsets_(recv_sizes_.size() + 1),
@@ -225,7 +225,7 @@ neighborhood_communicator::neighborhood_communicator(
 
 
 #define GKO_DECLARE_NEIGHBORHOOD_CONSTRUCTOR(LocalIndexType, GlobalIndexType) \
- neighborhood_communicator::neighborhood_communicator( \
+ NeighborhoodCommunicator::NeighborhoodCommunicator(  \
  communicator base, \
  const distributed::index_map<LocalIndexType, GlobalIndexType>& imap)
 

core/test/mpi/distributed/neighborhood_communicator.cpp

@@ -25,7 +25,7 @@ class NeighborhoodCommunicator : public ::testing::Test {
 
 TEST_F(NeighborhoodCommunicator, CanDefaultConstruct)
 {
- gko::experimental::mpi::neighborhood_communicator nhcomm{comm};
+ gko::experimental::mpi::NeighborhoodCommunicator nhcomm{comm};
 
  ASSERT_EQ(nhcomm.get_base_communicator(), comm);
  ASSERT_EQ(nhcomm.get_send_size(), 0);
@@ -45,7 +45,7 @@ TEST_F(NeighborhoodCommunicator, CanConstructFromIndexMap)
  {ref, {8, 12, 13, 14}}};
  auto imap = map_type{ref, part, comm.rank(), recv_connections[rank]};
 
- gko::experimental::mpi::neighborhood_communicator spcomm{comm, imap};
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{comm, imap};
 
  std::array<gko::size_type, 6> send_sizes = {4, 6, 2, 4, 7, 3};
  ASSERT_EQ(spcomm.get_recv_size(), recv_connections[rank].get_size());
@@ -69,7 +69,7 @@ TEST_F(NeighborhoodCommunicator, CanConstructFromEnvelopData)
  std::partial_sum(send_sizes[rank].begin(), send_sizes[rank].end(),
  send_offsets.begin() + 1);
 
- gko::experimental::mpi::neighborhood_communicator spcomm{
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{
  comm, sources[rank], send_sizes[rank], send_offsets,
  destinations, recv_sizes[rank], recv_offsets};
 
@@ -82,7 +82,7 @@ TEST_F(NeighborhoodCommunicator, CanConstructFromEmptyIndexMap)
 {
  auto imap = map_type{ref};
 
- gko::experimental::mpi::neighborhood_communicator spcomm{comm, imap};
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{comm, imap};
 
  ASSERT_EQ(spcomm.get_recv_size(), 0);
  ASSERT_EQ(spcomm.get_send_size(), 0);
@@ -95,7 +95,7 @@ TEST_F(NeighborhoodCommunicator, CanConstructFromIndexMapWithoutConnection)
  ref, comm.size(), comm.size() * 3));
  auto imap = map_type{ref, part, comm.rank(), {ref, 0}};
 
- gko::experimental::mpi::neighborhood_communicator spcomm{comm, imap};
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{comm, imap};
 
  ASSERT_EQ(spcomm.get_recv_size(), 0);
  ASSERT_EQ(spcomm.get_send_size(), 0);
@@ -111,7 +111,7 @@ TEST_F(NeighborhoodCommunicator, CanConstructFromEmptyEnvelopData)
  std::vector<comm_index_type> recv_offsets{0};
  std::vector<comm_index_type> send_offsets{0};
 
- gko::experimental::mpi::neighborhood_communicator spcomm{
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{
  comm, sources, send_sizes, send_offsets,
  destinations, recv_sizes, recv_offsets};
 
@@ -131,7 +131,7 @@ TEST_F(NeighborhoodCommunicator, CanCommunicateIalltoall)
  {ref, {4, 5, 9, 10, 16, 15}},
  {ref, {8, 12, 13, 14}}};
  auto imap = map_type{ref, part, comm.rank(), recv_connections[rank]};
- gko::experimental::mpi::neighborhood_communicator spcomm{comm, imap};
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{comm, imap};
  gko::array<long> recv_buffer{ref, recv_connections[rank].get_size()};
  gko::array<long> send_buffers[] = {{ref, {0, 1, 1, 2}},
  {ref, {3, 5, 3, 4, 4, 5}},
@@ -150,7 +150,7 @@ TEST_F(NeighborhoodCommunicator, CanCommunicateIalltoall)
 
 TEST_F(NeighborhoodCommunicator, CanCommunicateIalltoallWhenEmpty)
 {
- gko::experimental::mpi::neighborhood_communicator spcomm{comm};
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{comm};
 
  auto req = spcomm.i_all_to_all_v(ref, static_cast<int*>(nullptr),
  static_cast<int*>(nullptr));
@@ -169,7 +169,7 @@ TEST_F(NeighborhoodCommunicator, CanCreateInverse)
  {ref, {4, 5, 9, 10, 16, 15}},
  {ref, {8, 12, 13, 14}}};
  auto imap = map_type{ref, part, comm.rank(), recv_connections[rank]};
- gko::experimental::mpi::neighborhood_communicator spcomm{comm, imap};
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{comm, imap};
 
  auto inverse = spcomm.create_inverse();
 
@@ -189,7 +189,7 @@ TEST_F(NeighborhoodCommunicator, CanCommunicateRoundTrip)
  {ref, {4, 5, 9, 10, 16, 15}},
  {ref, {8, 12, 13, 14}}};
  auto imap = map_type{ref, part, comm.rank(), recv_connections[rank]};
- gko::experimental::mpi::neighborhood_communicator spcomm{comm, imap};
+ gko::experimental::mpi::NeighborhoodCommunicator spcomm{comm, imap};
  auto inverse = spcomm.create_inverse();
  gko::array<long> send_buffers[] = {{ref, {1, 2, 3, 4}},
  {ref, {5, 6, 7, 8, 9, 10}},

include/ginkgo/core/base/types.hpp

@@ -813,7 +813,7 @@ namespace distributed {
 
 
 /**
- * Make mpi::comm_index_type avaiable in this namespace
+ * Make mpi::comm_index_type available in this namespace
  */
 using mpi::comm_index_type;
 

include/ginkgo/core/distributed/collective_communicator.hpp

@@ -25,11 +25,11 @@ namespace mpi {
  * A collective communicator only provides routines for collective
  * communications. At the moment this is restricted to the variable all-to-all.
  */
-class collective_communicator {
+class CollectiveCommunicator {
 public:
- virtual ~collective_communicator() = default;
+ virtual ~CollectiveCommunicator() = default;
 
- explicit collective_communicator(communicator base = MPI_COMM_NULL)
+ explicit CollectiveCommunicator(communicator base = MPI_COMM_NULL)
  : base_(std::move(base))
  {}
 
@@ -41,8 +41,9 @@ class collective_communicator {
  /**
  * Non-blocking all-to-all communication.
  *
- * The send_buffer must have size get_send_size, and the recv_buffer
- * must have size get_recv_size.
+ * The send_buffer must have allocated at least get_send_size number of
+ * elements, and the recv_buffer must have allocated at least get_recv_size
+ * number of elements.
  *
  * @tparam SendType the type of the elements to send
  * @tparam RecvType the type of the elements to receive
@@ -78,7 +79,7 @@ class collective_communicator {
  *
  * @return a collective_communicator with the same dynamic type
  */
- [[nodiscard]] virtual std::unique_ptr<collective_communicator>
+ [[nodiscard]] virtual std::unique_ptr<CollectiveCommunicator>
  create_with_same_type(communicator base,
  const distributed::index_map_variant& imap) const = 0;
 
@@ -89,7 +90,7 @@ class collective_communicator {
  * @return a collective_communicator with the inverse communication
  * pattern.
  */
- [[nodiscard]] virtual std::unique_ptr<collective_communicator>
+ [[nodiscard]] virtual std::unique_ptr<CollectiveCommunicator>
  create_inverse() const = 0;
 
  /**

include/ginkgo/core/distributed/index_map_fwd.hpp

@@ -2,8 +2,8 @@
 //
 // SPDX-License-Identifier: BSD-3-Clause
 
-#ifndef GINKGO_INDEX_MAP_FWD_HPP
-#define GINKGO_INDEX_MAP_FWD_HPP
+#ifndef GKO_PUBLIC_CORE_INDEX_MAP_FWD_HPP
+#define GKO_PUBLIC_CORE_INDEX_MAP_FWD_HPP
 
 #include <variant>
 
@@ -27,4 +27,4 @@ using index_map_variant =
 } // namespace experimental
 } // namespace gko
 
-#endif // GINKGO_INDEX_MAP_FWD_HPP
+#endif // GKO_PUBLIC_CORE_INDEX_MAP_FWD_HPP

include/ginkgo/core/distributed/neighborhood_communicator.hpp

@@ -29,15 +29,15 @@ namespace mpi {
  * No communication with any ranks that is not in one of those lists will
  * take place.
  */
-class neighborhood_communicator final : public collective_communicator {
+class NeighborhoodCommunicator final : public CollectiveCommunicator {
 public:
- using collective_communicator::i_all_to_all_v;
+ using CollectiveCommunicator::i_all_to_all_v;
 
  /**
  * Default constructor with empty communication pattern
  * @param base the base communicator
  */
- explicit neighborhood_communicator(communicator base);
+ explicit NeighborhoodCommunicator(communicator base);
 
  /**
  * Create a neighborhood_communicator from an index map.
@@ -52,7 +52,7 @@ class neighborhood_communicator final : public collective_communicator {
  * @param imap the index map that defines the communication pattern
  */
  template <typename LocalIndexType, typename GlobalIndexType>
- neighborhood_communicator(
+ NeighborhoodCommunicator(
  communicator base,
  const distributed::index_map<LocalIndexType, GlobalIndexType>& imap);
 
@@ -68,7 +68,7 @@ class neighborhood_communicator final : public collective_communicator {
  * @param send_sizes the number of elements to send for each destination
  * @param send_offsets the offset for each destination
  */
- neighborhood_communicator(
+ NeighborhoodCommunicator(
  communicator base,
  const std::vector<distributed::comm_index_type>& sources,
  const std::vector<comm_index_type>& recv_sizes,
@@ -78,43 +78,38 @@ class neighborhood_communicator final : public collective_communicator {
  const std::vector<comm_index_type>& send_offsets);
 
  /**
- * Communicate data from all ranks to all other ranks using the
- * neighboorhood communication MPI_Ineighbor_alltoallv. See MPI
- * documentation for more details
+ * @copydoc CollectiveCommunicator::i_all_to_all_v
  *
- * @param exec The executor, on which the message buffers are located.
- * @param send_buffer the buffer to send
- * @param send_type the MPI_Datatype for the send buffer
- * @param recv_buffer the buffer to gather into
- * @param recv_type the MPI_Datatype for the recv buffer
- *
- * @return the request handle for the call
+ * This implementation uses the neighborhood communication
+ * MPI_Ineighbor_alltoallv. See MPI documentation for more details.
  */
  request i_all_to_all_v(std::shared_ptr<const Executor> exec,
  const void* send_buffer, MPI_Datatype send_type,
  void* recv_buffer,
  MPI_Datatype recv_type) const override;
 
- std::unique_ptr<collective_communicator> create_with_same_type(
+ [[nodiscard]] std::unique_ptr<CollectiveCommunicator> create_with_same_type(
  communicator base,
  const distributed::index_map_variant& imap) const override;
+
  /**
  * Creates the inverse neighborhood_communicator by switching sources
  * and destinations.
  *
  * @return collective_communicator with the inverse communication pattern
  */
- std::unique_ptr<collective_communicator> create_inverse() const override;
+ [[nodiscard]] std::unique_ptr<CollectiveCommunicator> create_inverse()
+ const override;
 
  /**
  * @copydoc collective_communicator::get_recv_size
  */
- comm_index_type get_recv_size() const override;
+ [[nodiscard]] comm_index_type get_recv_size() const override;
 
  /**
  * @copydoc collective_communicator::get_recv_size
  */
- comm_index_type get_send_size() const override;
+ [[nodiscard]] comm_index_type get_send_size() const override;
 
 private:
  communicator comm_;