[coll-comm] review updates:

- documentation - formatting Co-authored-by: Yu-Hsiang M. Tsai <[email protected]> Signed-off-by: Marcel Koch <[email protected]>
ginkgo-project · Dec 18, 2024 · 807118c · 807118c
1 parent 30a8b3e
commit 807118c
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 50 deletions.
diff --git a/core/distributed/collective_communicator.cpp b/core/distributed/collective_communicator.cpp
@@ -4,6 +4,8 @@
 
 #include "ginkgo/core/distributed/collective_communicator.hpp"
 
+#include <mpi.h>
+
 
 namespace gko {
 namespace experimental {

diff --git a/core/distributed/neighborhood_communicator.cpp b/core/distributed/neighborhood_communicator.cpp
@@ -14,27 +14,27 @@ namespace mpi {
 
 
 /**
- * \brief Computes the inverse envelope (target-ids, sizes) for a given
+ * @brief Computes the inverse envelope (target-ids, sizes) for a given
  *        one-sided communication pattern.
  *
- * \param exec the executor, this will always use the host executor
- * \param comm communicator
- * \param ids target ids of the one-sided operation
- * \param sizes number of elements send to each id
+ * @param exec the executor, this will always use the host executor
+ * @param comm communicator
+ * @param ids target ids of the one-sided operation
+ * @param sizes number of elements send to each id
  *
- * \return the inverse envelope consisting of the target-ids and the sizes
+ * @return the inverse envelope consisting of the target-ids and the sizes
  */
 std::tuple<std::vector<comm_index_type>, std::vector<comm_index_type>>
 communicate_inverse_envelope(std::shared_ptr<const Executor> exec,
-                             mpi::communicator comm,
+                             communicator comm,
                              const std::vector<comm_index_type>& ids,
                              const std::vector<comm_index_type>& sizes)
 {
     auto host_exec = exec->get_master();
     std::vector<comm_index_type> inverse_sizes_full(comm.size());
-    mpi::window<comm_index_type> window(host_exec, inverse_sizes_full.data(),
-                                        inverse_sizes_full.size(), comm,
-                                        sizeof(comm_index_type), MPI_INFO_ENV);
+    window<comm_index_type> window(host_exec, inverse_sizes_full.data(),
+                                   inverse_sizes_full.size(), comm,
+                                   sizeof(comm_index_type), MPI_INFO_ENV);
     window.fence();
     for (int i = 0; i < ids.size(); ++i) {
         window.put(host_exec, sizes.data() + i, 1, ids[i], comm.rank(), 1);
@@ -61,8 +61,8 @@ communicate_inverse_envelope(std::shared_ptr<const Executor> exec,
  * The graph is unweighted and has the same rank ordering as the input
  * communicator.
  */
-mpi::communicator create_neighborhood_comm(
-    mpi::communicator base, const std::vector<comm_index_type>& sources,
+communicator create_neighborhood_comm(
+    communicator base, const std::vector<comm_index_type>& sources,
     const std::vector<comm_index_type>& destinations)
 {
     auto in_degree = static_cast<comm_index_type>(sources.size());
@@ -80,8 +80,7 @@ mpi::communicator create_neighborhood_comm(
         info, false, &graph_comm));
     GKO_ASSERT_NO_MPI_ERRORS(MPI_Info_free(&info));
 
-    return mpi::communicator::create_owning(graph_comm,
-                                            base.force_host_buffer());
+    return communicator::create_owning(graph_comm, base.force_host_buffer());
 }
 
 

diff --git a/include/ginkgo/core/base/mpi.hpp b/include/ginkgo/core/base/mpi.hpp
@@ -457,10 +457,9 @@ class communicator {
     /**
      * Creates a new communicator and takes ownership of the MPI_Comm.
      *
-     * The ownership is shared with all communicators that are created as a
-     * copy from the new communicator.
-     * The underlying MPI_Comm will be freed when the last communicator with
-     * ownership is destroyed.
+     * The ownership is shared with all mpi::communicator objects that are a
+     * copy from the newly created communicator. The underlying MPI_Comm will be
+     * freed when the last communicator with ownership is destroyed.
      *
      * @see communicator(const MPI_Comm&, bool)
      */

diff --git a/include/ginkgo/core/distributed/collective_communicator.hpp b/include/ginkgo/core/distributed/collective_communicator.hpp
@@ -11,6 +11,7 @@
 
 #if GINKGO_BUILD_MPI
 
+
 #include <ginkgo/core/base/mpi.hpp>
 #include <ginkgo/core/distributed/index_map_fwd.hpp>
 
@@ -19,8 +20,9 @@ namespace gko {
 namespace experimental {
 namespace mpi {
 
+
 /**
- * Interface for an collective communicator.
+ * Interface for a collective communicator.
  *
  * A collective communicator only provides routines for collective
  * communications. At the moment this is restricted to the variable all-to-all.
@@ -42,6 +44,7 @@ class CollectiveCommunicator {
      *
      * @tparam SendType  the type of the elements to send
      * @tparam RecvType  the type of the elements to receive
+     *
      * @param exec  the executor for the communication
      * @param send_buffer  the send buffer
      * @param recv_buffer  the receive buffer
@@ -61,23 +64,23 @@ class CollectiveCommunicator {
                            void* recv_buffer, MPI_Datatype recv_type) const;
 
     /**
-     * Creates a new collective_communicator with the same dynamic type.
+     * Creates a new CollectiveCommunicator with the same dynamic type.
      *
      * @param base  The base communicator
      * @param imap  The index_map that defines the communication pattern
      *
-     * @return  a collective_communicator with the same dynamic type
+     * @return  a CollectiveCommunicator with the same dynamic type
      */
     [[nodiscard]] virtual std::unique_ptr<CollectiveCommunicator>
     create_with_same_type(communicator base,
                           const distributed::index_map_variant& imap) const = 0;
 
     /**
-     * Creates a collective_communicator with the inverse communication pattern
+     * Creates a CollectiveCommunicator with the inverse communication pattern
      * than this object.
      *
-     * @return  a collective_communicator with the inverse communication
-     * pattern.
+     * @return  a CollectiveCommunicator with the inverse communication
+     *          pattern.
      */
     [[nodiscard]] virtual std::unique_ptr<CollectiveCommunicator>
     create_inverse() const = 0;
@@ -125,5 +128,6 @@ request CollectiveCommunicator::i_all_to_all_v(
 }  // namespace experimental
 }  // namespace gko
 
+
 #endif
 #endif  // GKO_PUBLIC_CORE_DISTRIBUTED_COLLECTIVE_COMMUNICATOR_HPP_
diff --git a/include/ginkgo/core/distributed/dense_communicator.hpp b/include/ginkgo/core/distributed/dense_communicator.hpp
@@ -11,6 +11,7 @@
 
 #if GINKGO_BUILD_MPI
 
+
 #include <ginkgo/core/base/mpi.hpp>
 #include <ginkgo/core/distributed/collective_communicator.hpp>
 #include <ginkgo/core/distributed/index_map.hpp>
@@ -22,7 +23,7 @@ namespace mpi {
 
 
 /**
- * A collective_communicator that uses a dense communication.
+ * A CollectiveCommunicator that uses a dense communication.
  *
  * The dense communicator uses the MPI_Alltoall function for its communication.
  */
@@ -40,19 +41,21 @@ class DenseCommunicator final : public CollectiveCommunicator {
 
     /**
      * Default constructor with empty communication pattern
+     *
      * @param base  the base communicator
      */
     explicit DenseCommunicator(communicator base);
 
     /**
      * Create a DenseCommunicator from an index map.
      *
-     * The receive neighbors are defined by the remote indices and their
+     * The receiving neighbors are defined by the remote indices and their
      * owning ranks of the index map. The send neighbors are deduced
      * from that through collective communication.
      *
      * @tparam LocalIndexType  the local index type of the map
      * @tparam GlobalIndexType  the global index type of the map
+     *
      * @param base  the base communicator
      * @param imap  the index map that defines the communication pattern
      */
@@ -61,9 +64,6 @@ class DenseCommunicator final : public CollectiveCommunicator {
         communicator base,
         const distributed::index_map<LocalIndexType, GlobalIndexType>& imap);
 
-    /**
-     * @copydoc 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;
@@ -72,22 +72,15 @@ class DenseCommunicator final : public CollectiveCommunicator {
      * Creates the inverse DenseCommunicator by switching sources
      * and destinations.
      *
-     * @return  collective_communicator with the inverse communication pattern
+     * @return  CollectiveCommunicator with the inverse communication pattern
      */
     [[nodiscard]] std::unique_ptr<CollectiveCommunicator> create_inverse()
         const override;
 
-    /**
-     * @copydoc collective_communicator::get_recv_size
-     */
     [[nodiscard]] comm_index_type get_recv_size() const override;
 
-    /**
-     * @copydoc collective_communicator::get_recv_size
-     */
     [[nodiscard]] comm_index_type get_send_size() const override;
 
-
     /**
      * Compares two communicators for equality.
      *
@@ -134,5 +127,6 @@ class DenseCommunicator final : public CollectiveCommunicator {
 }  // namespace experimental
 }  // namespace gko
 
+
 #endif
 #endif  // GKO_PUBLIC_CORE_DISTRIBUTED_DENSE_COMMUNICATOR_HPP_
diff --git a/include/ginkgo/core/distributed/neighborhood_communicator.hpp b/include/ginkgo/core/distributed/neighborhood_communicator.hpp
@@ -11,6 +11,7 @@
 
 #if GINKGO_BUILD_MPI
 
+
 #include <ginkgo/core/base/mpi.hpp>
 #include <ginkgo/core/distributed/collective_communicator.hpp>
 #include <ginkgo/core/distributed/index_map.hpp>
@@ -22,7 +23,7 @@ namespace mpi {
 
 
 /**
- * A collective_communicator that uses a neighborhood topology.
+ * A CollectiveCommunicator that uses a neighborhood topology.
  *
  * The neighborhood communicator is defined by a list of neighbors this
  * rank sends data to and a list of neighbors this rank receives data from.
@@ -44,19 +45,21 @@ class NeighborhoodCommunicator final : public CollectiveCommunicator {
 
     /**
      * Default constructor with empty communication pattern
+     *
      * @param base  the base communicator
      */
     explicit NeighborhoodCommunicator(communicator base);
 
     /**
      * Create a NeighborhoodCommunicator from an index map.
      *
-     * The receive neighbors are defined by the remote indices and their
+     * The receiving neighbors are defined by the remote indices and their
      * owning ranks of the index map. The send neighbors are deduced
      * from that through collective communication.
      *
      * @tparam LocalIndexType  the local index type of the map
      * @tparam GlobalIndexType  the global index type of the map
+     *
      * @param base  the base communicator
      * @param imap  the index map that defines the communication pattern
      */
@@ -65,9 +68,6 @@ class NeighborhoodCommunicator final : public CollectiveCommunicator {
         communicator base,
         const distributed::index_map<LocalIndexType, GlobalIndexType>& imap);
 
-    /**
-     * @copydoc collective_communicator::create_with_same_type
-     */
     std::unique_ptr<CollectiveCommunicator> create_with_same_type(
         communicator base,
         const distributed::index_map_variant& imap) const override;
@@ -76,19 +76,13 @@ class NeighborhoodCommunicator final : public CollectiveCommunicator {
      * Creates the inverse NeighborhoodCommunicator by switching sources
      * and destinations.
      *
-     * @return  collective_communicator with the inverse communication pattern
+     * @return  CollectiveCommunicator with the inverse communication pattern
      */
     [[nodiscard]] std::unique_ptr<CollectiveCommunicator> create_inverse()
         const override;
 
-    /**
-     * @copydoc collective_communicator::get_recv_size
-     */
     [[nodiscard]] comm_index_type get_recv_size() const override;
 
-    /**
-     * @copydoc collective_communicator::get_recv_size
-     */
     [[nodiscard]] comm_index_type get_send_size() const override;
 
     /**
@@ -123,7 +117,6 @@ class NeighborhoodCommunicator final : public CollectiveCommunicator {
                                 void* recv_buffer,
                                 MPI_Datatype recv_type) const override;
 
-
 private:
     communicator comm_;
 
@@ -138,5 +131,6 @@ class NeighborhoodCommunicator final : public CollectiveCommunicator {
 }  // namespace experimental
 }  // namespace gko
 
+
 #endif
 #endif  // GKO_PUBLIC_CORE_DISTRIBUTED_NEIGHBORHOOD_COMMUNICATOR_HPP_