From 059b9a82d182d6ea63eec05c8b90339db8e77705 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Sun, 12 Feb 2023 20:01:04 +0100 Subject: [PATCH 01/16] Add vec mean implementation Add vec mean tests --- .../matrix/dense_kernels.instantiate.cpp | 2 + .../unified/matrix/dense_kernels.template.cpp | 16 +++++++ core/device_hooks/common_kernels.inc.cpp | 1 + core/distributed/vector.cpp | 45 +++++++++++++++++-- core/matrix/dense.cpp | 33 ++++++++++++++ core/matrix/dense_kernels.hpp | 7 +++ include/ginkgo/core/distributed/vector.hpp | 26 +++++++++++ include/ginkgo/core/matrix/dense.hpp | 29 ++++++++++++ reference/matrix/dense_kernels.cpp | 22 +++++++++ reference/test/matrix/dense_kernels.cpp | 16 +++++++ test/mpi/vector.cpp | 21 +++++++++ 11 files changed, 215 insertions(+), 3 deletions(-) diff --git a/common/unified/matrix/dense_kernels.instantiate.cpp b/common/unified/matrix/dense_kernels.instantiate.cpp index bf20c8a19b6..f34d05954c4 100644 --- a/common/unified/matrix/dense_kernels.instantiate.cpp +++ b/common/unified/matrix/dense_kernels.instantiate.cpp @@ -99,6 +99,8 @@ GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE( // split GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE( GKO_DECLARE_DENSE_COMPUTE_SQUARED_NORM2_KERNEL); +// split +GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_MEAN_KERNEL); // end diff --git a/common/unified/matrix/dense_kernels.template.cpp b/common/unified/matrix/dense_kernels.template.cpp index b6ed5fb37e0..d7e1c08f38c 100644 --- a/common/unified/matrix/dense_kernels.template.cpp +++ b/common/unified/matrix/dense_kernels.template.cpp @@ -278,6 +278,22 @@ void compute_norm1(std::shared_ptr exec, } +template +void compute_mean(std::shared_ptr exec, + const matrix::Dense* x, + matrix::Dense* result, array& tmp) +{ + using ValueType_nc = gko::remove_complex; + run_kernel_col_reduction_cached( + exec, + [] GKO_KERNEL(auto i, auto j, auto x, auto total_size) { + return x(i, j) / static_cast(total_size); + }, + GKO_KERNEL_REDUCE_SUM(ValueType), result->get_values(), x->get_size(), + tmp, x, x->get_size()[0]); +} + + template void compute_max_nnz_per_row(std::shared_ptr exec, const matrix::Dense* source, diff --git a/core/device_hooks/common_kernels.inc.cpp b/core/device_hooks/common_kernels.inc.cpp index 87cab3dcf0b..e9cd938f9c2 100644 --- a/core/device_hooks/common_kernels.inc.cpp +++ b/core/device_hooks/common_kernels.inc.cpp @@ -331,6 +331,7 @@ GKO_STUB_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_CONJ_DOT_DISPATCH_KERNEL); GKO_STUB_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_NORM2_KERNEL); GKO_STUB_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_NORM2_DISPATCH_KERNEL); GKO_STUB_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_NORM1_KERNEL); +GKO_STUB_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_MEAN_KERNEL); GKO_STUB_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_SQUARED_NORM2_KERNEL); GKO_STUB_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_SQRT_KERNEL); GKO_STUB_VALUE_AND_INDEX_TYPE(GKO_DECLARE_DENSE_FILL_IN_MATRIX_DATA_KERNEL); diff --git a/core/distributed/vector.cpp b/core/distributed/vector.cpp index 001cf75b76d..f05a2df73fd 100644 --- a/core/distributed/vector.cpp +++ b/core/distributed/vector.cpp @@ -30,10 +30,8 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. *************************************************************/ -#include - - #include +#include #include "core/distributed/vector_kernels.hpp" @@ -573,12 +571,52 @@ void Vector::compute_squared_norm2(ptr_param result, } +template +void Vector::compute_mean(LinOp* result) const +{ + array tmp{this->get_executor()}; + this->compute_mean(result, tmp); +} + + +void Vector::compute_mean(LinOp* result, array& tmp) const +{ + using MeanVector = local_vector_type; + const auto global_size = this->get_size()[0]; + const auto local_size = this->get_local_vector()->get_size()[0]; + const auto num_vecs = static_cast(this->get_size()[1]); + GKO_ASSERT_EQUAL_DIMENSIONS(result, dim<2>(1, num_vecs)); + auto exec = this->get_executor(); + const auto comm = this->get_communicator(); + auto dense_res = make_temporary_clone(exec, as(result)); + this->get_local_vector()->compute_mean(dense_res.get()); + + // scale by its weight ie ratio of local to global size + auto weight = initialize>>( + 1, {static_cast>(local_size) / global_size}, + this->get_executor()); + dense_res->scale(weight.get()); + + exec->synchronize(); + if (mpi::requires_host_buffer(exec, comm)) { + host_reduction_buffer_.init(exec->get_master(), dense_res->get_size()); + host_reduction_buffer_->copy_from(dense_res.get()); + comm.all_reduce(exec->get_master(), + host_reduction_buffer_->get_values(), num_vecs, + MPI_SUM); + dense_res->copy_from(host_reduction_buffer_.get()); + } else { + comm.all_reduce(exec, dense_res->get_values(), num_vecs, MPI_SUM); + } +} + template ValueType& Vector::at_local(size_type row, size_type col) noexcept { return local_.at(row, col); } + template ValueType Vector::at_local(size_type row, size_type col) const noexcept @@ -586,6 +624,7 @@ ValueType Vector::at_local(size_type row, return local_.at(row, col); } + template ValueType& Vector::at_local(size_type idx) noexcept { diff --git a/core/matrix/dense.cpp b/core/matrix/dense.cpp index 17dec93c234..babb1919040 100644 --- a/core/matrix/dense.cpp +++ b/core/matrix/dense.cpp @@ -80,6 +80,7 @@ GKO_REGISTER_OPERATION(compute_dot, dense::compute_dot_dispatch); GKO_REGISTER_OPERATION(compute_conj_dot, dense::compute_conj_dot_dispatch); GKO_REGISTER_OPERATION(compute_norm2, dense::compute_norm2_dispatch); GKO_REGISTER_OPERATION(compute_norm1, dense::compute_norm1); +GKO_REGISTER_OPERATION(compute_mean, dense::compute_mean); GKO_REGISTER_OPERATION(compute_squared_norm2, dense::compute_squared_norm2); GKO_REGISTER_OPERATION(compute_sqrt, dense::compute_sqrt); GKO_REGISTER_OPERATION(compute_max_nnz_per_row, dense::compute_max_nnz_per_row); @@ -235,6 +236,14 @@ void Dense::compute_squared_norm2(ptr_param result) const } +template +void Dense::compute_mean(LinOp* result) const +{ + auto exec = this->get_executor(); + this->compute_mean_impl(make_temporary_output_clone(exec, result).get()); +} + + template void Dense::inv_scale_impl(const LinOp* alpha) { @@ -496,6 +505,20 @@ void Dense::compute_squared_norm2(ptr_param result, } +template +void Dense::compute_mean(LinOp* result, array& tmp) const +{ + GKO_ASSERT_EQUAL_DIMENSIONS(result, dim<2>(1, this->get_size()[1])); + auto exec = this->get_executor(); + if (tmp.get_executor() != exec) { + tmp.clear(); + tmp.set_executor(exec); + } + auto dense_res = make_temporary_conversion(result); + exec->run(dense::make_compute_mean(this, dense_res.get(), tmp)); +} + + template void Dense::compute_squared_norm2_impl(LinOp* result) const { @@ -505,6 +528,16 @@ void Dense::compute_squared_norm2_impl(LinOp* result) const tmp); } +template +void Dense::compute_mean_impl(LinOp* result) const +{ + GKO_ASSERT_EQUAL_DIMENSIONS(result, dim<2>(1, this->get_size()[1])); + auto exec = this->get_executor(); + auto dense_res = make_temporary_conversion(result); + array tmp{exec}; + exec->run(dense::make_compute_mean(this, dense_res.get(), tmp)); +} + template Dense& Dense::operator=(const Dense& other) diff --git a/core/matrix/dense_kernels.hpp b/core/matrix/dense_kernels.hpp index 9a487fadeda..a352aa8d7c1 100644 --- a/core/matrix/dense_kernels.hpp +++ b/core/matrix/dense_kernels.hpp @@ -146,6 +146,11 @@ namespace kernels { matrix::Dense>* result, \ array& tmp) +#define GKO_DECLARE_DENSE_COMPUTE_MEAN_KERNEL(_type) \ + void compute_mean(std::shared_ptr exec, \ + const matrix::Dense<_type>* x, \ + matrix::Dense<_type>* result, array& tmp) + #define GKO_DECLARE_DENSE_FILL_IN_MATRIX_DATA_KERNEL(_type, _prec) \ void fill_in_matrix_data(std::shared_ptr exec, \ const device_matrix_data<_type, _prec>& data, \ @@ -349,6 +354,8 @@ namespace kernels { GKO_DECLARE_DENSE_COMPUTE_NORM2_DISPATCH_KERNEL(ValueType); \ template \ GKO_DECLARE_DENSE_COMPUTE_NORM1_KERNEL(ValueType); \ + template \ + GKO_DECLARE_DENSE_COMPUTE_MEAN_KERNEL(ValueType); \ template \ GKO_DECLARE_DENSE_FILL_IN_MATRIX_DATA_KERNEL(ValueType, IndexType); \ template \ diff --git a/include/ginkgo/core/distributed/vector.hpp b/include/ginkgo/core/distributed/vector.hpp index 61ceab8e380..e86c2ec3e61 100644 --- a/include/ginkgo/core/distributed/vector.hpp +++ b/include/ginkgo/core/distributed/vector.hpp @@ -404,6 +404,32 @@ class Vector */ void compute_norm1(ptr_param result, array& tmp) const; + /** + * Computes the column-wise mean of this (multi-)vector using a global + * reduction. + * + * @param result a Dense row matrix, used to store the mean + * (the number of columns in result must match the number + * of columns of this) + * @param tmp the temporary storage to use for partial sums during the + * reduction computation. It may be resized and/or reset to the + * correct executor. + */ + void compute_mean(LinOp* result) const; + + /** + * Computes the column-wise mean of this (multi-)vector using a global + * reduction. + * + * @param result a Dense row matrix, used to store the mean + * (the number of columns in result must match the number + * of columns of this) + * @param tmp the temporary storage to use for partial sums during the + * reduction computation. It may be resized and/or reset to the + * correct executor. + */ + void compute_mean(LinOp* result, array& tmp) const; + /** * Returns a single element of the multi-vector. * diff --git a/include/ginkgo/core/matrix/dense.hpp b/include/ginkgo/core/matrix/dense.hpp index ae738d49b93..1cba8622fce 100644 --- a/include/ginkgo/core/matrix/dense.hpp +++ b/include/ginkgo/core/matrix/dense.hpp @@ -917,6 +917,27 @@ class Dense */ void compute_squared_norm2(ptr_param result, array& tmp) const; + /** + * Computes the column-wise mean of this matrix. + * + * @param result a Dense row vector, used to store the norm + * (the number of columns in the vector must match the number + * of columns of this) + */ + void compute_mean(LinOp* result) const; + + /** + * Computes the column-wise mean of this matrix. + * + * @param result a Dense row vector, used to store the norm + * (the number of columns in the vector must match the + * number of columns of this) + * @param tmp the temporary storage to use for partial sums during the + * reduction computation. It may be resized and/or reset to the + * correct executor. + */ + void compute_mean(LinOp* result, array& tmp) const; + /** * Create a submatrix from the original matrix. * Warning: defining stride for this create_submatrix method might cause @@ -1215,6 +1236,14 @@ class Dense */ virtual void compute_squared_norm2_impl(LinOp* result) const; + /** + * @copydoc compute_mean(LinOp*) const + * + * @deprecated This function will be removed in the future, + * we will instead always use Ginkgo's implementation. + */ + virtual void compute_mean_impl(LinOp* result) const; + /** * Resizes the matrix to the given size. * diff --git a/reference/matrix/dense_kernels.cpp b/reference/matrix/dense_kernels.cpp index ba399b0f445..df86aedd047 100644 --- a/reference/matrix/dense_kernels.cpp +++ b/reference/matrix/dense_kernels.cpp @@ -397,6 +397,28 @@ void compute_norm1(std::shared_ptr exec, GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_NORM1_KERNEL); +template +void compute_mean(std::shared_ptr exec, + const matrix::Dense* x, + matrix::Dense* result, array&) +{ + using ValueType_nc = gko::remove_complex; + for (size_type j = 0; j < x->get_size()[1]; ++j) { + result->at(0, j) = zero(); + } + + for (size_type i = 0; i < x->get_size()[0]; ++i) { + const ValueType_nc alpha = static_cast(i) / (i + 1); + const ValueType_nc beta = static_cast(1) / (i + 1); + for (size_type j = 0; j < x->get_size()[1]; ++j) { + result->at(0, j) = alpha * result->at(0, j) + beta * x->at(i, j); + } + } +} + +GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE(GKO_DECLARE_DENSE_COMPUTE_MEAN_KERNEL); + + template void fill_in_matrix_data(std::shared_ptr exec, const device_matrix_data& data, diff --git a/reference/test/matrix/dense_kernels.cpp b/reference/test/matrix/dense_kernels.cpp index 9edab89e382..9a472262c22 100644 --- a/reference/test/matrix/dense_kernels.cpp +++ b/reference/test/matrix/dense_kernels.cpp @@ -700,6 +700,22 @@ TYPED_TEST(Dense, ComputesNorm1Mixed) } +TYPED_TEST(Dense, ComputesMean) +{ + using Mtx = typename TestFixture::Mtx; + using T = typename TestFixture::value_type; + using MeanVector = gko::matrix::Dense; + auto mtx(gko::initialize( + {I{1.0, 0.0}, I{2.0, 3.0}, I{2.0, 4.0}, I{-1.0, -1.0}}, + this->exec)); + auto result = MeanVector::create(this->exec, gko::dim<2>{1, 2}); + + mtx->compute_mean(result.get()); + + GKO_ASSERT_MTX_NEAR(result, l({{1.0, 1.5}}), 1e-2); +} + + TYPED_TEST(Dense, ComputeDotFailsOnWrongInputSize) { using Mtx = typename TestFixture::Mtx; diff --git a/test/mpi/vector.cpp b/test/mpi/vector.cpp index a7ad735458c..414f8197f57 100644 --- a/test/mpi/vector.cpp +++ b/test/mpi/vector.cpp @@ -675,6 +675,27 @@ TYPED_TEST(VectorReductions, ComputeSquaredNorm2WithTmpIsSameAsDense) r::value); } +TYPED_TEST(VectorReductions, ComputesMeanIsSameAsDense) +{ + using value_type = typename TestFixture::value_type; + this->init_result(); + + this->x->compute_mean(this->res.get()); + this->dense_x->compute_mean(this->dense_res.get()); + + GKO_ASSERT_MTX_NEAR(this->res, this->dense_res, r::value); +} + +TYPED_TEST(VectorReductions, ComputesMeanWithTmpIsSameAsDense) +{ + using value_type = typename TestFixture::value_type; + this->init_result(); + + this->x->compute_mean(this->res.get(), this->tmp); + this->dense_x->compute_mean(this->dense_res.get(), this->dense_tmp); + + GKO_ASSERT_MTX_NEAR(this->res, this->dense_res, r::value); +} TYPED_TEST(VectorReductions, ComputeDotCopiesToHostOnlyIfNecessary) { From e2faba19c0f47bc01a9a4a77bff09b6ddc4d6b2e Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Fri, 13 Oct 2023 20:44:03 +0200 Subject: [PATCH 02/16] use ptr_param --- core/distributed/vector.cpp | 5 +++-- include/ginkgo/core/distributed/vector.hpp | 12 +++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/core/distributed/vector.cpp b/core/distributed/vector.cpp index f05a2df73fd..23a6774ccd2 100644 --- a/core/distributed/vector.cpp +++ b/core/distributed/vector.cpp @@ -572,14 +572,15 @@ void Vector::compute_squared_norm2(ptr_param result, template -void Vector::compute_mean(LinOp* result) const +void Vector::compute_mean(ptr_param result) const { array tmp{this->get_executor()}; this->compute_mean(result, tmp); } -void Vector::compute_mean(LinOp* result, array& tmp) const +void Vector::compute_mean(ptr_param result, + array& tmp) const { using MeanVector = local_vector_type; const auto global_size = this->get_size()[0]; diff --git a/include/ginkgo/core/distributed/vector.hpp b/include/ginkgo/core/distributed/vector.hpp index e86c2ec3e61..86a82a2f7da 100644 --- a/include/ginkgo/core/distributed/vector.hpp +++ b/include/ginkgo/core/distributed/vector.hpp @@ -404,6 +404,16 @@ class Vector */ void compute_norm1(ptr_param result, array& tmp) const; + /** + * Computes the column-wise mean of this (multi-)vector using a global + * reduction. + * + * @param result a Dense row matrix, used to store the mean + * (the number of columns in result must match the number + * of columns of this) + */ + void compute_mean(ptr_param result) const; + /** * Computes the column-wise mean of this (multi-)vector using a global * reduction. @@ -415,7 +425,7 @@ class Vector * reduction computation. It may be resized and/or reset to the * correct executor. */ - void compute_mean(LinOp* result) const; + void compute_mean(ptr_param result, array& tmp) const; /** * Computes the column-wise mean of this (multi-)vector using a global From fd54ec392f36d69075d1a9f3815196277783a2d0 Mon Sep 17 00:00:00 2001 From: ginkgo-bot Date: Fri, 13 Oct 2023 19:06:23 +0000 Subject: [PATCH 03/16] Format files Co-authored-by: Gregor Olenik --- core/distributed/vector.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/distributed/vector.cpp b/core/distributed/vector.cpp index 23a6774ccd2..a67dc4f930e 100644 --- a/core/distributed/vector.cpp +++ b/core/distributed/vector.cpp @@ -30,10 +30,12 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. *************************************************************/ -#include #include +#include + + #include "core/distributed/vector_kernels.hpp" #include "core/matrix/dense_kernels.hpp" From edeecb06eddfcc0f6daac0a412a748cc9f09a935 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Sat, 14 Oct 2023 08:16:02 +0200 Subject: [PATCH 04/16] fix documentation --- include/ginkgo/core/matrix/dense.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/ginkgo/core/matrix/dense.hpp b/include/ginkgo/core/matrix/dense.hpp index 1cba8622fce..fb9773e1247 100644 --- a/include/ginkgo/core/matrix/dense.hpp +++ b/include/ginkgo/core/matrix/dense.hpp @@ -920,7 +920,7 @@ class Dense /** * Computes the column-wise mean of this matrix. * - * @param result a Dense row vector, used to store the norm + * @param result a Dense row vector, used to store the mean * (the number of columns in the vector must match the number * of columns of this) */ @@ -929,7 +929,7 @@ class Dense /** * Computes the column-wise mean of this matrix. * - * @param result a Dense row vector, used to store the norm + * @param result a Dense row vector, used to store the mean * (the number of columns in the vector must match the * number of columns of this) * @param tmp the temporary storage to use for partial sums during the From 5faa364ea0843f9779b16584f652cb4924b3575d Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Sat, 14 Oct 2023 14:56:03 +0200 Subject: [PATCH 05/16] update documentation and tests --- include/ginkgo/core/matrix/dense.hpp | 4 ++-- reference/test/matrix/dense_kernels.cpp | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/ginkgo/core/matrix/dense.hpp b/include/ginkgo/core/matrix/dense.hpp index fb9773e1247..912e857611c 100644 --- a/include/ginkgo/core/matrix/dense.hpp +++ b/include/ginkgo/core/matrix/dense.hpp @@ -918,7 +918,7 @@ class Dense void compute_squared_norm2(ptr_param result, array& tmp) const; /** - * Computes the column-wise mean of this matrix. + * Computes the column-wise arithmetic mean of this matrix. * * @param result a Dense row vector, used to store the mean * (the number of columns in the vector must match the number @@ -927,7 +927,7 @@ class Dense void compute_mean(LinOp* result) const; /** - * Computes the column-wise mean of this matrix. + * Computes the column-wise arithmetic mean of this matrix. * * @param result a Dense row vector, used to store the mean * (the number of columns in the vector must match the diff --git a/reference/test/matrix/dense_kernels.cpp b/reference/test/matrix/dense_kernels.cpp index 9a472262c22..1988c192444 100644 --- a/reference/test/matrix/dense_kernels.cpp +++ b/reference/test/matrix/dense_kernels.cpp @@ -704,6 +704,7 @@ TYPED_TEST(Dense, ComputesMean) { using Mtx = typename TestFixture::Mtx; using T = typename TestFixture::value_type; + using T_nc = gko::remove_complex; using MeanVector = gko::matrix::Dense; auto mtx(gko::initialize( {I{1.0, 0.0}, I{2.0, 3.0}, I{2.0, 4.0}, I{-1.0, -1.0}}, @@ -712,7 +713,8 @@ TYPED_TEST(Dense, ComputesMean) mtx->compute_mean(result.get()); - GKO_ASSERT_MTX_NEAR(result, l({{1.0, 1.5}}), 1e-2); + EXPECT_EQ(result->at(0, 0), T_nc{1.0}); + EXPECT_EQ(result->at(0, 1), T_nc{1.5}); } From 775c03064fd5c3ce9b4d900fc2623a6419612bd7 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Sat, 14 Oct 2023 15:24:05 +0200 Subject: [PATCH 06/16] use GKO_EXPECT_NEAR --- core/matrix/dense.cpp | 20 +++++++++++--------- include/ginkgo/core/matrix/dense.hpp | 4 ++-- reference/test/matrix/dense_kernels.cpp | 14 +++++--------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/core/matrix/dense.cpp b/core/matrix/dense.cpp index babb1919040..a50ab6b260b 100644 --- a/core/matrix/dense.cpp +++ b/core/matrix/dense.cpp @@ -236,14 +236,6 @@ void Dense::compute_squared_norm2(ptr_param result) const } -template -void Dense::compute_mean(LinOp* result) const -{ - auto exec = this->get_executor(); - this->compute_mean_impl(make_temporary_output_clone(exec, result).get()); -} - - template void Dense::inv_scale_impl(const LinOp* alpha) { @@ -506,7 +498,16 @@ void Dense::compute_squared_norm2(ptr_param result, template -void Dense::compute_mean(LinOp* result, array& tmp) const +void Dense::compute_mean(ptr_param result) const +{ + auto exec = this->get_executor(); + this->compute_mean_impl(make_temporary_output_clone(exec, result).get()); +} + + +template +void Dense::compute_mean(ptr_param result, + array& tmp) const { GKO_ASSERT_EQUAL_DIMENSIONS(result, dim<2>(1, this->get_size()[1])); auto exec = this->get_executor(); @@ -528,6 +529,7 @@ void Dense::compute_squared_norm2_impl(LinOp* result) const tmp); } + template void Dense::compute_mean_impl(LinOp* result) const { diff --git a/include/ginkgo/core/matrix/dense.hpp b/include/ginkgo/core/matrix/dense.hpp index 912e857611c..9edf55d2e4c 100644 --- a/include/ginkgo/core/matrix/dense.hpp +++ b/include/ginkgo/core/matrix/dense.hpp @@ -924,7 +924,7 @@ class Dense * (the number of columns in the vector must match the number * of columns of this) */ - void compute_mean(LinOp* result) const; + void compute_mean(ptr_param result) const; /** * Computes the column-wise arithmetic mean of this matrix. @@ -936,7 +936,7 @@ class Dense * reduction computation. It may be resized and/or reset to the * correct executor. */ - void compute_mean(LinOp* result, array& tmp) const; + void compute_mean(ptr_param result, array& tmp) const; /** * Create a submatrix from the original matrix. diff --git a/reference/test/matrix/dense_kernels.cpp b/reference/test/matrix/dense_kernels.cpp index 1988c192444..a033c1e1e3a 100644 --- a/reference/test/matrix/dense_kernels.cpp +++ b/reference/test/matrix/dense_kernels.cpp @@ -704,17 +704,13 @@ TYPED_TEST(Dense, ComputesMean) { using Mtx = typename TestFixture::Mtx; using T = typename TestFixture::value_type; - using T_nc = gko::remove_complex; - using MeanVector = gko::matrix::Dense; - auto mtx(gko::initialize( - {I{1.0, 0.0}, I{2.0, 3.0}, I{2.0, 4.0}, I{-1.0, -1.0}}, - this->exec)); - auto result = MeanVector::create(this->exec, gko::dim<2>{1, 2}); + auto result = Mtx::create(this->exec, gko::dim<2>{1, 3}); - mtx->compute_mean(result.get()); + this->mtx4->compute_mean(result.get()); - EXPECT_EQ(result->at(0, 0), T_nc{1.0}); - EXPECT_EQ(result->at(0, 1), T_nc{1.5}); + GKO_EXPECT_NEAR(result->at(0, 0), T{0.5}, 1e-6); + GKO_EXPECT_NEAR(result->at(0, 1), T{4.0}, 1e-6); + GKO_EXPECT_NEAR(result->at(0, 2), T{1.0}, 1e-6); } From a6276e469b6f8f6775182b1054ee3098ac22a960 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Sat, 14 Oct 2023 17:28:06 +0200 Subject: [PATCH 07/16] fix documentation --- include/ginkgo/core/distributed/vector.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/ginkgo/core/distributed/vector.hpp b/include/ginkgo/core/distributed/vector.hpp index 86a82a2f7da..1e3b9571b19 100644 --- a/include/ginkgo/core/distributed/vector.hpp +++ b/include/ginkgo/core/distributed/vector.hpp @@ -415,8 +415,8 @@ class Vector void compute_mean(ptr_param result) const; /** - * Computes the column-wise mean of this (multi-)vector using a global - * reduction. + * Computes the column-wise arithmetic mean of this (multi-)vector using a + * global reduction. * * @param result a Dense row matrix, used to store the mean * (the number of columns in result must match the number @@ -428,8 +428,8 @@ class Vector void compute_mean(ptr_param result, array& tmp) const; /** - * Computes the column-wise mean of this (multi-)vector using a global - * reduction. + * Computes the column-wise arithmetic mean of this (multi-)vector using a + * global reduction. * * @param result a Dense row matrix, used to store the mean * (the number of columns in result must match the number From 385822bfbe8ef2e739ce0320c14f6852709b36cf Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Sun, 15 Oct 2023 08:10:54 +0200 Subject: [PATCH 08/16] Fixup missing template declaration, add more tests --- core/distributed/vector.cpp | 1 + reference/test/matrix/dense_kernels.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/core/distributed/vector.cpp b/core/distributed/vector.cpp index a67dc4f930e..0693dc9106d 100644 --- a/core/distributed/vector.cpp +++ b/core/distributed/vector.cpp @@ -581,6 +581,7 @@ void Vector::compute_mean(ptr_param result) const } +template void Vector::compute_mean(ptr_param result, array& tmp) const { diff --git a/reference/test/matrix/dense_kernels.cpp b/reference/test/matrix/dense_kernels.cpp index a033c1e1e3a..f33ae2c6e14 100644 --- a/reference/test/matrix/dense_kernels.cpp +++ b/reference/test/matrix/dense_kernels.cpp @@ -714,6 +714,16 @@ TYPED_TEST(Dense, ComputesMean) } +TYPED_TEST(Dense, ComputesMeanFailsOnWrongResultSize) +{ + using Mtx = typename TestFixture::Mtx; + using T = typename TestFixture::value_type; + auto result = Mtx::create(this->exec, gko::dim<2>{1, 2}); + + ASSERT_THROW(this->mtx4->compute_mean(result), gko::DimensionMismatch); +} + + TYPED_TEST(Dense, ComputeDotFailsOnWrongInputSize) { using Mtx = typename TestFixture::Mtx; From 454adf7901343a53503a52b70a909a84411292a9 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Sun, 15 Oct 2023 12:31:48 +0200 Subject: [PATCH 09/16] Fixup call with ptr_param --- test/mpi/vector.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/mpi/vector.cpp b/test/mpi/vector.cpp index 414f8197f57..43d18aad6c5 100644 --- a/test/mpi/vector.cpp +++ b/test/mpi/vector.cpp @@ -680,8 +680,8 @@ TYPED_TEST(VectorReductions, ComputesMeanIsSameAsDense) using value_type = typename TestFixture::value_type; this->init_result(); - this->x->compute_mean(this->res.get()); - this->dense_x->compute_mean(this->dense_res.get()); + this->x->compute_mean(this->res); + this->dense_x->compute_mean(this->dense_res); GKO_ASSERT_MTX_NEAR(this->res, this->dense_res, r::value); } @@ -691,8 +691,8 @@ TYPED_TEST(VectorReductions, ComputesMeanWithTmpIsSameAsDense) using value_type = typename TestFixture::value_type; this->init_result(); - this->x->compute_mean(this->res.get(), this->tmp); - this->dense_x->compute_mean(this->dense_res.get(), this->dense_tmp); + this->x->compute_mean(this->res, this->tmp); + this->dense_x->compute_mean(this->dense_res, this->dense_tmp); GKO_ASSERT_MTX_NEAR(this->res, this->dense_res, r::value); } From 1c347c4afed6e1cec7b0f68ba14d326438890be0 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Mon, 16 Oct 2023 15:04:10 +0200 Subject: [PATCH 10/16] Add review suggestions Co-authored-by: Marcel Koch --- common/unified/matrix/dense_kernels.template.cpp | 6 +++--- core/distributed/vector.cpp | 4 ++-- core/matrix/dense.cpp | 6 ++---- include/ginkgo/core/distributed/vector.hpp | 13 ------------- reference/test/matrix/dense_kernels.cpp | 6 +++--- 5 files changed, 10 insertions(+), 25 deletions(-) diff --git a/common/unified/matrix/dense_kernels.template.cpp b/common/unified/matrix/dense_kernels.template.cpp index d7e1c08f38c..e8751a896a0 100644 --- a/common/unified/matrix/dense_kernels.template.cpp +++ b/common/unified/matrix/dense_kernels.template.cpp @@ -286,11 +286,11 @@ void compute_mean(std::shared_ptr exec, using ValueType_nc = gko::remove_complex; run_kernel_col_reduction_cached( exec, - [] GKO_KERNEL(auto i, auto j, auto x, auto total_size) { - return x(i, j) / static_cast(total_size); + [] GKO_KERNEL(auto i, auto j, auto x, auto inv_total_size) { + return x(i, j) * inv_total_size; }, GKO_KERNEL_REDUCE_SUM(ValueType), result->get_values(), x->get_size(), - tmp, x, x->get_size()[0]); + tmp, x, 1. / x->get_size()[0]); } diff --git a/core/distributed/vector.cpp b/core/distributed/vector.cpp index 0693dc9106d..3c5bfcf2c1f 100644 --- a/core/distributed/vector.cpp +++ b/core/distributed/vector.cpp @@ -589,7 +589,7 @@ void Vector::compute_mean(ptr_param result, const auto global_size = this->get_size()[0]; const auto local_size = this->get_local_vector()->get_size()[0]; const auto num_vecs = static_cast(this->get_size()[1]); - GKO_ASSERT_EQUAL_DIMENSIONS(result, dim<2>(1, num_vecs)); + GKO_ASSERT_EQUAL_COLS(result, dim<2>(1, num_vecs)); auto exec = this->get_executor(); const auto comm = this->get_communicator(); auto dense_res = make_temporary_clone(exec, as(result)); @@ -597,7 +597,7 @@ void Vector::compute_mean(ptr_param result, // scale by its weight ie ratio of local to global size auto weight = initialize>>( - 1, {static_cast>(local_size) / global_size}, + {static_cast>(local_size) / global_size}, this->get_executor()); dense_res->scale(weight.get()); diff --git a/core/matrix/dense.cpp b/core/matrix/dense.cpp index a50ab6b260b..68a26c5bd87 100644 --- a/core/matrix/dense.cpp +++ b/core/matrix/dense.cpp @@ -509,7 +509,7 @@ template void Dense::compute_mean(ptr_param result, array& tmp) const { - GKO_ASSERT_EQUAL_DIMENSIONS(result, dim<2>(1, this->get_size()[1])); + GKO_ASSERT_EQUAL_COLS(result, dim<2>(1, this->get_size()[1])); auto exec = this->get_executor(); if (tmp.get_executor() != exec) { tmp.clear(); @@ -533,11 +533,9 @@ void Dense::compute_squared_norm2_impl(LinOp* result) const template void Dense::compute_mean_impl(LinOp* result) const { - GKO_ASSERT_EQUAL_DIMENSIONS(result, dim<2>(1, this->get_size()[1])); auto exec = this->get_executor(); - auto dense_res = make_temporary_conversion(result); array tmp{exec}; - exec->run(dense::make_compute_mean(this, dense_res.get(), tmp)); + this->compute_mean(make_temporary_output_clone(exec, result).get(), tmp); } diff --git a/include/ginkgo/core/distributed/vector.hpp b/include/ginkgo/core/distributed/vector.hpp index 1e3b9571b19..87afa3a01b5 100644 --- a/include/ginkgo/core/distributed/vector.hpp +++ b/include/ginkgo/core/distributed/vector.hpp @@ -427,19 +427,6 @@ class Vector */ void compute_mean(ptr_param result, array& tmp) const; - /** - * Computes the column-wise arithmetic mean of this (multi-)vector using a - * global reduction. - * - * @param result a Dense row matrix, used to store the mean - * (the number of columns in result must match the number - * of columns of this) - * @param tmp the temporary storage to use for partial sums during the - * reduction computation. It may be resized and/or reset to the - * correct executor. - */ - void compute_mean(LinOp* result, array& tmp) const; - /** * Returns a single element of the multi-vector. * diff --git a/reference/test/matrix/dense_kernels.cpp b/reference/test/matrix/dense_kernels.cpp index f33ae2c6e14..51c0cec0548 100644 --- a/reference/test/matrix/dense_kernels.cpp +++ b/reference/test/matrix/dense_kernels.cpp @@ -708,9 +708,9 @@ TYPED_TEST(Dense, ComputesMean) this->mtx4->compute_mean(result.get()); - GKO_EXPECT_NEAR(result->at(0, 0), T{0.5}, 1e-6); - GKO_EXPECT_NEAR(result->at(0, 1), T{4.0}, 1e-6); - GKO_EXPECT_NEAR(result->at(0, 2), T{1.0}, 1e-6); + GKO_EXPECT_NEAR(result->at(0, 0), T{0.5}, r::value * 10); + GKO_EXPECT_NEAR(result->at(0, 1), T{4.0}, r::value * 10); + GKO_EXPECT_NEAR(result->at(0, 2), T{1.0}, r::value * 10); } From d3f973c7fd4d04b1cc4d3b82c88b7d96441246fc Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Tue, 17 Oct 2023 06:37:09 +0200 Subject: [PATCH 11/16] Update test/mpi/vector.cpp Co-authored-by: Yu-Hsiang M. Tsai <19565938+yhmtsai@users.noreply.github.com> --- test/mpi/vector.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/mpi/vector.cpp b/test/mpi/vector.cpp index 43d18aad6c5..515c8e59a7b 100644 --- a/test/mpi/vector.cpp +++ b/test/mpi/vector.cpp @@ -675,6 +675,7 @@ TYPED_TEST(VectorReductions, ComputeSquaredNorm2WithTmpIsSameAsDense) r::value); } + TYPED_TEST(VectorReductions, ComputesMeanIsSameAsDense) { using value_type = typename TestFixture::value_type; From 16ea4b4cb9df753db448f0ee8f41957dccdd39d9 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Tue, 17 Oct 2023 06:37:24 +0200 Subject: [PATCH 12/16] Update test/mpi/vector.cpp Co-authored-by: Yu-Hsiang M. Tsai <19565938+yhmtsai@users.noreply.github.com> --- common/unified/matrix/dense_kernels.template.cpp | 2 +- test/mpi/vector.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/common/unified/matrix/dense_kernels.template.cpp b/common/unified/matrix/dense_kernels.template.cpp index e8751a896a0..9bd5c04f861 100644 --- a/common/unified/matrix/dense_kernels.template.cpp +++ b/common/unified/matrix/dense_kernels.template.cpp @@ -290,7 +290,7 @@ void compute_mean(std::shared_ptr exec, return x(i, j) * inv_total_size; }, GKO_KERNEL_REDUCE_SUM(ValueType), result->get_values(), x->get_size(), - tmp, x, 1. / x->get_size()[0]); + tmp, x, ValueType_nc{1.} / x->get_size()[0]); } diff --git a/test/mpi/vector.cpp b/test/mpi/vector.cpp index 515c8e59a7b..2b00de19bda 100644 --- a/test/mpi/vector.cpp +++ b/test/mpi/vector.cpp @@ -687,6 +687,7 @@ TYPED_TEST(VectorReductions, ComputesMeanIsSameAsDense) GKO_ASSERT_MTX_NEAR(this->res, this->dense_res, r::value); } + TYPED_TEST(VectorReductions, ComputesMeanWithTmpIsSameAsDense) { using value_type = typename TestFixture::value_type; From fc6de3d57638e2aae97b0cafc0a8acf928999e1d Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Tue, 17 Oct 2023 06:39:03 +0200 Subject: [PATCH 13/16] Add review suggestions Co-authored-by: Marcel Koch --- core/distributed/vector.cpp | 2 +- core/matrix/dense.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/distributed/vector.cpp b/core/distributed/vector.cpp index 3c5bfcf2c1f..8b6b60f0963 100644 --- a/core/distributed/vector.cpp +++ b/core/distributed/vector.cpp @@ -589,7 +589,7 @@ void Vector::compute_mean(ptr_param result, const auto global_size = this->get_size()[0]; const auto local_size = this->get_local_vector()->get_size()[0]; const auto num_vecs = static_cast(this->get_size()[1]); - GKO_ASSERT_EQUAL_COLS(result, dim<2>(1, num_vecs)); + GKO_ASSERT_EQUAL_COLS(result, this); auto exec = this->get_executor(); const auto comm = this->get_communicator(); auto dense_res = make_temporary_clone(exec, as(result)); diff --git a/core/matrix/dense.cpp b/core/matrix/dense.cpp index 68a26c5bd87..9f7dff96aab 100644 --- a/core/matrix/dense.cpp +++ b/core/matrix/dense.cpp @@ -509,7 +509,7 @@ template void Dense::compute_mean(ptr_param result, array& tmp) const { - GKO_ASSERT_EQUAL_COLS(result, dim<2>(1, this->get_size()[1])); + GKO_ASSERT_EQUAL_COLS(result, this); auto exec = this->get_executor(); if (tmp.get_executor() != exec) { tmp.clear(); From 7b15bdb4a61b682537eb3ead4a41790882201d64 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Tue, 17 Oct 2023 08:53:50 +0200 Subject: [PATCH 14/16] Doc fixes and format --- include/ginkgo/core/matrix/dense.hpp | 3 --- test/mpi/vector.cpp | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/include/ginkgo/core/matrix/dense.hpp b/include/ginkgo/core/matrix/dense.hpp index 9edf55d2e4c..0db8f7697a5 100644 --- a/include/ginkgo/core/matrix/dense.hpp +++ b/include/ginkgo/core/matrix/dense.hpp @@ -1238,9 +1238,6 @@ class Dense /** * @copydoc compute_mean(LinOp*) const - * - * @deprecated This function will be removed in the future, - * we will instead always use Ginkgo's implementation. */ virtual void compute_mean_impl(LinOp* result) const; diff --git a/test/mpi/vector.cpp b/test/mpi/vector.cpp index 2b00de19bda..ac75a461465 100644 --- a/test/mpi/vector.cpp +++ b/test/mpi/vector.cpp @@ -699,6 +699,7 @@ TYPED_TEST(VectorReductions, ComputesMeanWithTmpIsSameAsDense) GKO_ASSERT_MTX_NEAR(this->res, this->dense_res, r::value); } + TYPED_TEST(VectorReductions, ComputeDotCopiesToHostOnlyIfNecessary) { this->init_result(); From d152e7313af1819677cfd3c821d766c00738fa1a Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Tue, 17 Oct 2023 08:54:30 +0200 Subject: [PATCH 15/16] Use simpler implementation for reference --- reference/matrix/dense_kernels.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/reference/matrix/dense_kernels.cpp b/reference/matrix/dense_kernels.cpp index df86aedd047..ff69dcf2684 100644 --- a/reference/matrix/dense_kernels.cpp +++ b/reference/matrix/dense_kernels.cpp @@ -408,11 +408,10 @@ void compute_mean(std::shared_ptr exec, } for (size_type i = 0; i < x->get_size()[0]; ++i) { - const ValueType_nc alpha = static_cast(i) / (i + 1); - const ValueType_nc beta = static_cast(1) / (i + 1); for (size_type j = 0; j < x->get_size()[1]; ++j) { - result->at(0, j) = alpha * result->at(0, j) + beta * x->at(i, j); + result->at(0, i) += x->at(i, j); } + result->at(0, i) /= static_cast(x->get_size()[1]); } } From ed9c4b71cbc06cb4a5918c620e7d5614dc68baa1 Mon Sep 17 00:00:00 2001 From: Gregor Olenik Date: Wed, 18 Oct 2023 10:09:42 +0200 Subject: [PATCH 16/16] Fix reference compute mean impl, add test --- reference/matrix/dense_kernels.cpp | 8 ++++---- reference/test/matrix/dense_kernels.cpp | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/reference/matrix/dense_kernels.cpp b/reference/matrix/dense_kernels.cpp index ff69dcf2684..47df46b3c86 100644 --- a/reference/matrix/dense_kernels.cpp +++ b/reference/matrix/dense_kernels.cpp @@ -407,11 +407,11 @@ void compute_mean(std::shared_ptr exec, result->at(0, j) = zero(); } - for (size_type i = 0; i < x->get_size()[0]; ++i) { - for (size_type j = 0; j < x->get_size()[1]; ++j) { - result->at(0, i) += x->at(i, j); + for (size_type i = 0; i < x->get_size()[1]; ++i) { + for (size_type j = 0; j < x->get_size()[0]; ++j) { + result->at(0, i) += x->at(j, i); } - result->at(0, i) /= static_cast(x->get_size()[1]); + result->at(0, i) /= static_cast(x->get_size()[0]); } } diff --git a/reference/test/matrix/dense_kernels.cpp b/reference/test/matrix/dense_kernels.cpp index 51c0cec0548..ef46dab77a9 100644 --- a/reference/test/matrix/dense_kernels.cpp +++ b/reference/test/matrix/dense_kernels.cpp @@ -35,6 +35,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include #include @@ -704,6 +705,13 @@ TYPED_TEST(Dense, ComputesMean) { using Mtx = typename TestFixture::Mtx; using T = typename TestFixture::value_type; + + auto iota = Mtx::create(this->exec, gko::dim<2>{10, 1}); + std::iota(iota->get_values(), iota->get_values() + 10, 1); + auto iota_result = Mtx::create(this->exec, gko::dim<2>{1, 1}); + iota->compute_mean(iota_result.get()); + GKO_EXPECT_NEAR(iota_result->at(0, 0), T{5.5}, r::value * 10); + auto result = Mtx::create(this->exec, gko::dim<2>{1, 3}); this->mtx4->compute_mean(result.get());