From ceb6f2e7e86ed11eb4b98b99a6187555fdfa7fe3 Mon Sep 17 00:00:00 2001 From: Marcel Koch Date: Tue, 17 Dec 2024 14:56:46 +0100 Subject: [PATCH] [coll-comm] review updates: - documentation - formatting Co-authored-by: Yu-Hsiang M. Tsai Signed-off-by: Marcel Koch --- core/distributed/collective_communicator.cpp | 2 ++ .../distributed/neighborhood_communicator.cpp | 27 +++++++++---------- include/ginkgo/core/base/mpi.hpp | 7 +++-- .../distributed/collective_communicator.hpp | 16 ++++++----- .../core/distributed/dense_communicator.hpp | 20 +++++--------- .../distributed/neighborhood_communicator.hpp | 20 +++++--------- 6 files changed, 42 insertions(+), 50 deletions(-) diff --git a/core/distributed/collective_communicator.cpp b/core/distributed/collective_communicator.cpp index a8336f02241..34539e45091 100644 --- a/core/distributed/collective_communicator.cpp +++ b/core/distributed/collective_communicator.cpp @@ -4,6 +4,8 @@ #include "ginkgo/core/distributed/collective_communicator.hpp" +#include + namespace gko { namespace experimental { diff --git a/core/distributed/neighborhood_communicator.cpp b/core/distributed/neighborhood_communicator.cpp index 287595f5b15..2fbbebf0e47 100644 --- 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> communicate_inverse_envelope(std::shared_ptr exec, - mpi::communicator comm, + communicator comm, const std::vector& ids, const std::vector& sizes) { auto host_exec = exec->get_master(); std::vector inverse_sizes_full(comm.size()); - mpi::window window(host_exec, inverse_sizes_full.data(), - inverse_sizes_full.size(), comm, - sizeof(comm_index_type), MPI_INFO_ENV); + window 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 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& sources, +communicator create_neighborhood_comm( + communicator base, const std::vector& sources, const std::vector& destinations) { auto in_degree = static_cast(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 index 10242b1c4ca..40b229c3b50 100644 --- 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 index d62bbdb2783..0e049f989fd 100644 --- 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 #include @@ -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 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 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 index 99975e1b661..9696672c98d 100644 --- 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 #include #include @@ -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,6 +41,7 @@ class DenseCommunicator final : public CollectiveCommunicator { /** * Default constructor with empty communication pattern + * * @param base the base communicator */ explicit DenseCommunicator(communicator base); @@ -47,12 +49,13 @@ class DenseCommunicator final : public CollectiveCommunicator { /** * 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& imap); - /** - * @copydoc collective_communicator::create_with_same_type - */ [[nodiscard]] std::unique_ptr 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 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 index 7b4bb50fd1e..94c451c6971 100644 --- 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 #include #include @@ -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,6 +45,7 @@ class NeighborhoodCommunicator final : public CollectiveCommunicator { /** * Default constructor with empty communication pattern + * * @param base the base communicator */ explicit NeighborhoodCommunicator(communicator base); @@ -51,12 +53,13 @@ class NeighborhoodCommunicator final : public CollectiveCommunicator { /** * 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& imap); - /** - * @copydoc collective_communicator::create_with_same_type - */ std::unique_ptr 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 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_