From 830d4a6d46a8d655ca8550d921adae447c3ab9e5 Mon Sep 17 00:00:00 2001 From: Pratik Nayak Date: Mon, 8 Nov 2021 12:11:58 +0100 Subject: [PATCH] Review update. Co-authored-by: Tobias Ribizel --- core/matrix/csr.cpp | 10 ++++++---- cuda/test/matrix/csr_kernels.cpp | 21 +++++++++++---------- hip/test/matrix/csr_kernels.hip.cpp | 21 +++++++++++---------- include/ginkgo/core/base/lin_op.hpp | 14 -------------- include/ginkgo/core/matrix/csr.hpp | 29 ++--------------------------- omp/test/matrix/csr_kernels.cpp | 15 +++++++++------ 6 files changed, 39 insertions(+), 71 deletions(-) diff --git a/core/matrix/csr.cpp b/core/matrix/csr.cpp index 0e015baabcb..f67b4e8667d 100644 --- a/core/matrix/csr.cpp +++ b/core/matrix/csr.cpp @@ -50,7 +50,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "core/components/absolute_array.hpp" #include "core/components/fill_array.hpp" #include "core/components/prefix_sum.hpp" -#include "core/components/reduce_array.hpp" #include "core/matrix/csr_kernels.hpp" @@ -90,7 +89,6 @@ GKO_REGISTER_OPERATION(is_sorted_by_column_index, csr::is_sorted_by_column_index); GKO_REGISTER_OPERATION(extract_diagonal, csr::extract_diagonal); GKO_REGISTER_OPERATION(fill_array, components::fill_array); -GKO_REGISTER_OPERATION(reduce_add_array, components::reduce_add_array); GKO_REGISTER_OPERATION(prefix_sum, components::prefix_sum); GKO_REGISTER_OPERATION(inplace_absolute_array, components::inplace_absolute_array); @@ -557,8 +555,12 @@ Csr::create_submatrix(const gko::span& row_span, exec->run(csr::make_calculate_nonzeros_per_row_in_span( this, row_span, column_span, &row_ptrs)); exec->run(csr::make_prefix_sum(row_ptrs.get_data(), row_span.length() + 1)); - auto sub_mat = Mat::create(exec, sub_mat_size, std::move(row_ptrs), - this->get_strategy()); + auto num_nnz = + exec->copy_val_to_host(row_ptrs.get_data() + sub_mat_size[0]); + auto sub_mat = Mat::create(exec, sub_mat_size, + std::move(Array(exec, num_nnz)), + std::move(Array(exec, num_nnz)), + std::move(row_ptrs), this->get_strategy()); exec->run(csr::make_compute_submatrix(this, row_span, column_span, sub_mat.get())); sub_mat->make_srow(); diff --git a/cuda/test/matrix/csr_kernels.cpp b/cuda/test/matrix/csr_kernels.cpp index a6f6843b71f..0100fc3c0f0 100644 --- a/cuda/test/matrix/csr_kernels.cpp +++ b/cuda/test/matrix/csr_kernels.cpp @@ -938,11 +938,10 @@ TEST_F(Csr, OutplaceAbsoluteComplexMatrixIsEquivalentToRef) } -TEST_F(Csr, CalculateNnzPerRowIsEquivalentToRef) +TEST_F(Csr, CalculateNnzPerRowInSpanIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; set_up_apply_data(); - gko::span rspan{7, 51}; gko::span cspan{22, 88}; auto size = this->mtx2->get_size(); @@ -962,8 +961,9 @@ TEST_F(Csr, CalculateNnzPerRowIsEquivalentToRef) TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; + using IndexType = int; + using ValueType = double; set_up_apply_data(); - gko::span rspan{7, 51}; gko::span cspan{22, 88}; auto size = this->mtx2->get_size(); @@ -973,12 +973,17 @@ TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) this->ref, this->mtx2.get(), rspan, cspan, &row_nnz); gko::kernels::reference::components::prefix_sum( this->ref, row_nnz.get_data(), row_nnz.get_num_elems()); + auto num_nnz = row_nnz.get_data()[rspan.length()]; auto drow_nnz = gko::Array(this->cuda, row_nnz); auto smat1 = Mtx::create(this->ref, gko::dim<2>(rspan.length(), cspan.length()), + std::move(gko::Array(this->ref, num_nnz)), + std::move(gko::Array(this->ref, num_nnz)), std::move(row_nnz)); auto sdmat1 = Mtx::create(this->cuda, gko::dim<2>(rspan.length(), cspan.length()), + std::move(gko::Array(this->cuda, num_nnz)), + std::move(gko::Array(this->cuda, num_nnz)), std::move(drow_nnz)); @@ -986,10 +991,8 @@ TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) rspan, cspan, smat1.get()); gko::kernels::cuda::csr::compute_submatrix(this->cuda, this->dmtx2.get(), rspan, cspan, sdmat1.get()); - auto hmat = Mtx::create(this->ref); - hmat->copy_from(sdmat1.get()); - GKO_ASSERT_MTX_NEAR(hmat, smat1, 0.0); + GKO_ASSERT_MTX_NEAR(sdmat1, smat1, 0.0); } @@ -997,15 +1000,13 @@ TEST_F(Csr, CreateSubMatrixIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; set_up_apply_data(); - gko::span rspan{47, 81}; gko::span cspan{2, 31}; + auto smat1 = this->mtx2->create_submatrix(rspan, cspan); auto sdmat1 = this->dmtx2->create_submatrix(rspan, cspan); - auto hmat = Mtx::create(this->ref); - hmat->copy_from(sdmat1.get()); - GKO_ASSERT_MTX_NEAR(hmat, smat1, 0.0); + GKO_ASSERT_MTX_NEAR(sdmat1, smat1, 0.0); } diff --git a/hip/test/matrix/csr_kernels.hip.cpp b/hip/test/matrix/csr_kernels.hip.cpp index 332f87e3837..3971227a475 100644 --- a/hip/test/matrix/csr_kernels.hip.cpp +++ b/hip/test/matrix/csr_kernels.hip.cpp @@ -948,11 +948,10 @@ TEST_F(Csr, OutplaceAbsoluteComplexMatrixIsEquivalentToRef) } -TEST_F(Csr, CalculateNnzPerRowIsEquivalentToRef) +TEST_F(Csr, CalculateNnzPerRowInSpanIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; set_up_apply_data(); - gko::span rspan{7, 51}; gko::span cspan{22, 88}; auto size = this->mtx2->get_size(); @@ -972,8 +971,9 @@ TEST_F(Csr, CalculateNnzPerRowIsEquivalentToRef) TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; + using IndexType = int; + using ValueType = double; set_up_apply_data(); - gko::span rspan{7, 51}; gko::span cspan{22, 88}; auto size = this->mtx2->get_size(); @@ -983,12 +983,17 @@ TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) this->ref, this->mtx2.get(), rspan, cspan, &row_nnz); gko::kernels::reference::components::prefix_sum( this->ref, row_nnz.get_data(), row_nnz.get_num_elems()); + auto num_nnz = row_nnz.get_data()[rspan.length()]; auto drow_nnz = gko::Array(this->hip, row_nnz); auto smat1 = Mtx::create(this->ref, gko::dim<2>(rspan.length(), cspan.length()), + std::move(gko::Array(this->ref, num_nnz)), + std::move(gko::Array(this->ref, num_nnz)), std::move(row_nnz)); auto sdmat1 = Mtx::create(this->hip, gko::dim<2>(rspan.length(), cspan.length()), + std::move(gko::Array(this->hip, num_nnz)), + std::move(gko::Array(this->hip, num_nnz)), std::move(drow_nnz)); @@ -996,10 +1001,8 @@ TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) rspan, cspan, smat1.get()); gko::kernels::hip::csr::compute_submatrix(this->hip, this->dmtx2.get(), rspan, cspan, sdmat1.get()); - auto hmat = Mtx::create(this->ref); - hmat->copy_from(sdmat1.get()); - GKO_ASSERT_MTX_NEAR(hmat, smat1, 0.0); + GKO_ASSERT_MTX_NEAR(sdmat1, smat1, 0.0); } @@ -1007,15 +1010,13 @@ TEST_F(Csr, CreateSubMatrixIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; set_up_apply_data(); - gko::span rspan{47, 81}; gko::span cspan{2, 31}; + auto smat1 = this->mtx2->create_submatrix(rspan, cspan); auto sdmat1 = this->dmtx2->create_submatrix(rspan, cspan); - auto hmat = Mtx::create(this->ref); - hmat->copy_from(sdmat1.get()); - GKO_ASSERT_MTX_NEAR(hmat, smat1, 0.0); + GKO_ASSERT_MTX_NEAR(sdmat1, smat1, 0.0); } diff --git a/include/ginkgo/core/base/lin_op.hpp b/include/ginkgo/core/base/lin_op.hpp index a43580c8d81..540fd6ebad3 100644 --- a/include/ginkgo/core/base/lin_op.hpp +++ b/include/ginkgo/core/base/lin_op.hpp @@ -654,20 +654,6 @@ class Preconditionable { }; -/** - * A submatrix of the LinOp implementing this interface can be extracted. - * The row and column spans must be in range of the size of the matrix. - * - * @ingroup LinOp - */ -template -class SubMatrixExtractable { -public: - virtual std::unique_ptr create_submatrix( - const gko::span& row_span, const gko::span& column_span) const = 0; -}; - - /** * The diagonal of a LinOp can be extracted. It will be implemented by * DiagonalExtractable, so the class does not need to implement it. diff --git a/include/ginkgo/core/matrix/csr.hpp b/include/ginkgo/core/matrix/csr.hpp index cfd8ab8dbe2..94f7384438a 100644 --- a/include/ginkgo/core/matrix/csr.hpp +++ b/include/ginkgo/core/matrix/csr.hpp @@ -127,7 +127,6 @@ class Csr : public EnableLinOp>, public ConvertibleTo>, public ConvertibleTo>, public DiagonalExtractable, - public SubMatrixExtractable>, public ReadableFromMatrixData, public WritableToMatrixData, public Transposable, @@ -765,9 +764,9 @@ class Csr : public EnableLinOp>, std::unique_ptr> extract_diagonal() const override; std::unique_ptr> create_submatrix( - const gko::span& row_span, const gko::span& column_span) const override; + const gko::span& row_span, const gko::span& column_span) const; - std::unique_ptr compute_absolute() const override; + std::unique_ptr compute_absolute() const; void compute_absolute_inplace() override; @@ -985,30 +984,6 @@ class Csr : public EnableLinOp>, strategy_(strategy->copy()) {} - /** - * Creates a CSR matrix from already allocated (and initialized) row - * pointer array. - * - * @param exec Executor associated to the matrix - * @param size size of the matrix - * @param row_ptrs array of row pointers - */ - explicit Csr( - std::shared_ptr exec, const dim<2>& size, - Array&& row_ptrs, - std::shared_ptr strategy = std::make_shared()) - : EnableLinOp(exec, size), - row_ptrs_{exec, std::move(row_ptrs)}, - srow_(exec), - strategy_(strategy->copy()) - { - GKO_ASSERT(row_ptrs_.get_num_elems() == size[0] + 1); - auto num_nnz = exec->copy_val_to_host(row_ptrs_.get_data() + size[0]); - values_ = Array(exec, num_nnz); - col_idxs_ = Array(exec, num_nnz); - this->make_srow(); - } - /** * Creates a CSR matrix from already allocated (and initialized) row * pointer, column index and value arrays. diff --git a/omp/test/matrix/csr_kernels.cpp b/omp/test/matrix/csr_kernels.cpp index 11bfdf1dced..d6712cfa7d6 100644 --- a/omp/test/matrix/csr_kernels.cpp +++ b/omp/test/matrix/csr_kernels.cpp @@ -711,11 +711,10 @@ TEST_F(Csr, OutplaceAbsoluteComplexMatrixIsEquivalentToRef) } -TEST_F(Csr, CalculateNnzPerRowIsEquivalentToRef) +TEST_F(Csr, CalculateNnzPerRowInSpanIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; set_up_mat_data(); - gko::span rspan{7, 51}; gko::span cspan{22, 88}; auto size = this->mtx2->get_size(); @@ -735,8 +734,9 @@ TEST_F(Csr, CalculateNnzPerRowIsEquivalentToRef) TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) { using Mtx = gko::matrix::Csr<>; + using IndexType = int; + using ValueType = double; set_up_mat_data(); - gko::span rspan{7, 51}; gko::span cspan{22, 88}; auto size = this->mtx2->get_size(); @@ -746,12 +746,17 @@ TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) this->ref, this->mtx2.get(), rspan, cspan, &row_nnz); gko::kernels::reference::components::prefix_sum( this->ref, row_nnz.get_data(), row_nnz.get_num_elems()); + auto num_nnz = row_nnz.get_data()[rspan.length()]; auto drow_nnz = gko::Array(this->omp, row_nnz); auto smat1 = Mtx::create(this->ref, gko::dim<2>(rspan.length(), cspan.length()), + std::move(gko::Array(this->ref, num_nnz)), + std::move(gko::Array(this->ref, num_nnz)), std::move(row_nnz)); auto sdmat1 = Mtx::create(this->omp, gko::dim<2>(rspan.length(), cspan.length()), + std::move(gko::Array(this->omp, num_nnz)), + std::move(gko::Array(this->omp, num_nnz)), std::move(drow_nnz)); @@ -759,10 +764,8 @@ TEST_F(Csr, ComputeSubmatrixIsEquivalentToRef) rspan, cspan, smat1.get()); gko::kernels::omp::csr::compute_submatrix(this->omp, this->dmtx2.get(), rspan, cspan, sdmat1.get()); - auto hmat = Mtx::create(this->ref); - hmat->copy_from(sdmat1.get()); - GKO_ASSERT_MTX_NEAR(hmat, smat1, 0.0); + GKO_ASSERT_MTX_NEAR(sdmat1, smat1, 0.0); }