From d0502788652e024bf3c5dd89a05c357df08b116e Mon Sep 17 00:00:00 2001 From: Marcel Koch Date: Fri, 25 Mar 2022 11:56:29 +0100 Subject: [PATCH] review updates Co-authored-by: Tobias Ribizel Co-authored-by: Pratik Nayak --- core/distributed/matrix.cpp | 16 ++++---- core/test/utils/matrix_generator.hpp | 2 +- include/ginkgo/core/distributed/matrix.hpp | 44 ++++++++++++++-------- omp/distributed/matrix_kernels.cpp | 12 +++--- omp/test/distributed/matrix_kernels.cpp | 6 +++ reference/distributed/matrix_kernels.cpp | 6 +-- 6 files changed, 51 insertions(+), 35 deletions(-) diff --git a/core/distributed/matrix.cpp b/core/distributed/matrix.cpp index 750837fbb4b..98d86bd87a6 100644 --- a/core/distributed/matrix.cpp +++ b/core/distributed/matrix.cpp @@ -83,15 +83,15 @@ Matrix::Matrix( gather_idxs_{exec}, local_to_global_ghost_{exec}, one_scalar_{}, - diag_mtx_{inner_matrix_type->create_default(exec)}, - offdiag_mtx_{ghost_matrix_type->create_default(exec)} + diag_mtx_{inner_matrix_type->clone(exec)}, + offdiag_mtx_{ghost_matrix_type->clone(exec)} { - GKO_ASSERT(GKO_QUOTE( - dynamic_cast*>( - diag_mtx.get()))); - GKO_ASSERT(GKO_QUOTE( - dynamic_cast*>( - offdiag_mtx.get()))); + GKO_ASSERT( + (dynamic_cast*>( + diag_mtx_.get()))); + GKO_ASSERT( + (dynamic_cast*>( + offdiag_mtx_.get()))); one_scalar_.init(exec, dim<2>{1, 1}); one_scalar_->fill(one()); } diff --git a/core/test/utils/matrix_generator.hpp b/core/test/utils/matrix_generator.hpp index e1289cdc3a0..e849724f871 100644 --- a/core/test/utils/matrix_generator.hpp +++ b/core/test/utils/matrix_generator.hpp @@ -106,7 +106,7 @@ matrix_data generate_random_matrix_data( /** - * Generates matrix data for a random matrix. + * Generates device matrix data for a random matrix. * * @see generate_random_matrix_data */ diff --git a/include/ginkgo/core/distributed/matrix.hpp b/include/ginkgo/core/distributed/matrix.hpp index dd384b277e5..810b63ef7f1 100644 --- a/include/ginkgo/core/distributed/matrix.hpp +++ b/include/ginkgo/core/distributed/matrix.hpp @@ -84,6 +84,16 @@ struct MatrixTypeBuilderFromValueAndIndex { }; +template +struct is_matrix_builder : std::false_type {}; + +template +struct is_matrix_builder< + T, xstd::void_t().template create( + std::declval>()))>> + : std::true_type {}; + + } // namespace detail @@ -140,7 +150,7 @@ class Vector; * * The matrix is stored in a row-wise distributed format. * Each process owns a specific set of rows, where the assignment of rows is - * defined by a row @see Partition. The following depicts the distribution of + * defined by a row Partition. The following depicts the distribution of * global rows according to their assigned part-id (which will usually be the * owning process id): * ``` @@ -161,14 +171,14 @@ class Vector; * following: * ``` * Part-Id Global Inner Ghost - * 0 | .. 1 : 2 .. : .. .. | | .. 1 | | 2 | - * 0 | 3 4 : .. .. : .. .. | | 3 4 | | .. | + * 0 | .. 1 ⁞ 2 .. ⁞ .. .. | | .. 1 | | 2 | + * 0 | 3 4 ⁞ .. .. ⁞ .. .. | | 3 4 | | .. | * |-----------------------| - * 1 | .. 5 : 6 .. : 7 .. | ----> | 6 .. | | 5 7 .. | - * 1 | .. .. : .. 8 : .. 9 | ----> | 8 .. | | .. .. 9 | + * 1 | .. 5 ⁞ 6 .. ⁞ 7 .. | ----> | 6 .. | | 5 7 .. | + * 1 | .. .. ⁞ .. 8 ⁞ .. 9 | ----> | 8 .. | | .. .. 9 | * |-----------------------| - * 2 | .. .. : .. 10 : 11 12 | | 11 12 | | .. 10 | - * 2 | 13 .. : .. .. : 14 .. | | 14 .. | | 13 .. | + * 2 | .. .. ⁞ .. 10 ⁞ 11 12 | | 11 12 | | .. 10 | + * 2 | 13 .. ⁞ .. .. ⁞ 14 .. | | 14 .. | | 13 .. | * ``` * This uses the same ownership of the columns as for the rows. * Additionally, the ownership of the columns may be explicitly defined with an @@ -189,13 +199,13 @@ class Vector; * ``` * * The Matrix should be filled using the read_distributed method, e.g. - * ```c++ + * ``` * auto part = Partition<...>::build_from_mapping(...); * auto mat = Matrix<...>::create(exec, comm); * mat->read_distributed(matrix_data, part); * ``` * or if different partitions for the rows and columns are used: - * ```c++ + * ``` * auto row_part = Partition<...>::build_from_mapping(...); * auto col_part = Partition<...>::build_from_mapping(...); * auto mat = Matrix<...>::create(exec, comm); @@ -368,7 +378,7 @@ class Matrix * @param comm Communicator associated with this matrix. * @param matrix_type the local matrices will be constructed with the same * type as `create` returns. It should be the return - * value of make_matrix_type. + * value of make_matrix_type. */ template explicit Matrix(std::shared_ptr exec, @@ -421,6 +431,10 @@ class Matrix /** * Creates an empty distributed matrix with specified type * for local matricies. + * + * @note It internally clones the passed in matrix_type. Therefore, this + * LinOp should be empty. + * * @param exec Executor associated with this matrix. * @param comm Communicator associated with this matrix. * @param matrix_type the local matrices will be constructed with the same @@ -433,8 +447,8 @@ class Matrix * Creates an empty distributed matrix with specified types for the local * inner matrix and the local ghost matrix. * - * @note It calls Matrix(std::shared_ptr,mpi::communicator, - * const LinOp*,const LinOp*) underneath + * @note It internally clones the passed in inner_matrix_type and + * ghost_matrix_type. Therefore, these LinOps should be empty. * * @tparam InnerMatrixType A type that has a `create(exec)` function to create an smart @@ -454,11 +468,11 @@ class Matrix const LinOp* ghost_matrix_type); /** - * Asynchronously communicates the values of b that are shared with other - * processors. + * Starts a non-blocking communication of the values of b that are shared + * with other processors. * @param local_b The full local vector to be communicated. The subset of * shared values is automatically extracted. - * @return MPI request for the async communication. + * @return MPI request for the non-blocking communication. */ mpi::request communicate(const local_vector_type* local_b) const; diff --git a/omp/distributed/matrix_kernels.cpp b/omp/distributed/matrix_kernels.cpp index 09c3b4a599a..bd6580ba3b3 100644 --- a/omp/distributed/matrix_kernels.cpp +++ b/omp/distributed/matrix_kernels.cpp @@ -89,8 +89,7 @@ void build_diag_offdiag( return static_cast(std::distance(range_bounds + 1, it)); } }; - auto map_to_local_row = [&](GlobalIndexType idx, - size_type range_id) -> LocalIndexType { + auto map_to_local_row = [&](GlobalIndexType idx, size_type range_id) { auto range_bounds = row_partition->get_range_bounds(); auto range_starting_indices = row_partition->get_range_starting_indices(); @@ -108,8 +107,7 @@ void build_diag_offdiag( return static_cast(std::distance(range_bounds + 1, it)); } }; - auto map_to_local_col = [&](GlobalIndexType idx, - size_type range_id) -> LocalIndexType { + auto map_to_local_col = [&](GlobalIndexType idx, size_type range_id) { auto range_bounds = col_partition->get_range_bounds(); auto range_starting_indices = col_partition->get_range_starting_indices(); @@ -118,10 +116,10 @@ void build_diag_offdiag( }; // store offdiagonal columns and their range indices - std::map offdiag_cols; + map offdiag_cols(exec); // store offdiagonal entries with global column idxs - std::vector global_offdiag_entries; - std::vector diag_entries; + vector global_offdiag_entries(exec); + vector diag_entries(exec); auto num_threads = static_cast(omp_get_max_threads()); auto num_input = input.get_num_elems(); diff --git a/omp/test/distributed/matrix_kernels.cpp b/omp/test/distributed/matrix_kernels.cpp index ecd623d08ab..fba0a0c21d8 100644 --- a/omp/test/distributed/matrix_kernels.cpp +++ b/omp/test/distributed/matrix_kernels.cpp @@ -151,6 +151,7 @@ TYPED_TEST(Matrix, BuildsDiagOffdiagEmptyIsSameAsRef) using global_index_type = typename TestFixture::global_index_type; gko::Array mapping{this->ref, {1, 0, 2, 2, 0, 1, 1, 2}}; comm_index_type num_parts = 3; + auto partition = gko::distributed::Partition< local_index_type, global_index_type>::build_from_mapping(this->ref, mapping, @@ -186,6 +187,7 @@ TYPED_TEST(Matrix, BuildsLocalSmallIsEquivalentToRef) std::uniform_int_distribution(0, static_cast(num_cols - 1)), std::uniform_real_distribution>(0, 1), this->engine, this->ref); + auto partition = gko::distributed::Partition< local_index_type, global_index_type>::build_from_mapping(this->ref, mapping, @@ -221,6 +223,7 @@ TYPED_TEST(Matrix, BuildsLocalIsEquivalentToRef) static_cast(num_cols - 1)), std::uniform_real_distribution>(0, 1), this->engine, this->ref); + auto partition = gko::distributed::Partition< local_index_type, global_index_type>::build_from_mapping(this->ref, mapping, @@ -245,6 +248,7 @@ TYPED_TEST(Matrix, BuildsDiagOffdiagEmptyWithColPartitionIsSameAsRef) gko::Array col_mapping{this->ref, {0, 0, 2, 2, 2, 1, 1, 1}}; comm_index_type num_parts = 3; + auto row_partition = gko::distributed::Partition< local_index_type, global_index_type>::build_from_mapping(this->ref, row_mapping, @@ -295,6 +299,7 @@ TYPED_TEST(Matrix, BuildsLocalSmallWithColPartitionIsEquivalentToRef) std::uniform_int_distribution(0, static_cast(num_cols - 1)), std::uniform_real_distribution>(0, 1), this->engine, this->ref); + auto row_partition = gko::distributed::Partition< local_index_type, global_index_type>::build_from_mapping(this->ref, row_mapping, @@ -344,6 +349,7 @@ TYPED_TEST(Matrix, BuildsLocalWithColPartitionIsEquivalentToRef) static_cast(num_cols - 1)), std::uniform_real_distribution>(0, 1), this->engine, this->ref); + auto row_partition = gko::distributed::Partition< local_index_type, global_index_type>::build_from_mapping(this->ref, row_mapping, diff --git a/reference/distributed/matrix_kernels.cpp b/reference/distributed/matrix_kernels.cpp index 56e61e42cbb..f428719c974 100644 --- a/reference/distributed/matrix_kernels.cpp +++ b/reference/distributed/matrix_kernels.cpp @@ -83,8 +83,7 @@ void build_diag_offdiag( return static_cast(std::distance(range_bounds + 1, it)); } }; - auto map_to_local_row = [&](GlobalIndexType idx, - size_type range_id) -> LocalIndexType { + auto map_to_local_row = [&](GlobalIndexType idx, size_type range_id) { auto range_bounds = row_partition->get_range_bounds(); auto range_starting_indices = row_partition->get_range_starting_indices(); @@ -102,8 +101,7 @@ void build_diag_offdiag( return static_cast(std::distance(range_bounds + 1, it)); } }; - auto map_to_local_col = [&](GlobalIndexType idx, - size_type range_id) -> LocalIndexType { + auto map_to_local_col = [&](GlobalIndexType idx, size_type range_id) { auto range_bounds = col_partition->get_range_bounds(); auto range_starting_indices = col_partition->get_range_starting_indices();