Skip to content

Commit

Permalink
Review update.
Browse files Browse the repository at this point in the history
Co-authored-by: Tobias Ribizel <ribizel@kit.edu>
  • Loading branch information
pratikvn and upsj committed Nov 8, 2021
1 parent fbfcee5 commit 830d4a6
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 71 deletions.
10 changes: 6 additions & 4 deletions core/matrix/csr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"


Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -557,8 +555,12 @@ Csr<ValueType, IndexType>::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<ValueType>(exec, num_nnz)),
std::move(Array<IndexType>(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();
Expand Down
21 changes: 11 additions & 10 deletions cuda/test/matrix/csr_kernels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -973,39 +973,40 @@ 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<int>(this->cuda, row_nnz);
auto smat1 =
Mtx::create(this->ref, gko::dim<2>(rspan.length(), cspan.length()),
std::move(gko::Array<ValueType>(this->ref, num_nnz)),
std::move(gko::Array<IndexType>(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<ValueType>(this->cuda, num_nnz)),
std::move(gko::Array<IndexType>(this->cuda, num_nnz)),
std::move(drow_nnz));


gko::kernels::reference::csr::compute_submatrix(this->ref, this->mtx2.get(),
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);
}


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);
}


Expand Down
21 changes: 11 additions & 10 deletions hip/test/matrix/csr_kernels.hip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -983,39 +983,40 @@ 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<int>(this->hip, row_nnz);
auto smat1 =
Mtx::create(this->ref, gko::dim<2>(rspan.length(), cspan.length()),
std::move(gko::Array<ValueType>(this->ref, num_nnz)),
std::move(gko::Array<IndexType>(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<ValueType>(this->hip, num_nnz)),
std::move(gko::Array<IndexType>(this->hip, num_nnz)),
std::move(drow_nnz));


gko::kernels::reference::csr::compute_submatrix(this->ref, this->mtx2.get(),
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);
}


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);
}


Expand Down
14 changes: 0 additions & 14 deletions include/ginkgo/core/base/lin_op.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename ConcreteType>
class SubMatrixExtractable {
public:
virtual std::unique_ptr<ConcreteType> 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<ValueType>, so the class does not need to implement it.
Expand Down
29 changes: 2 additions & 27 deletions include/ginkgo/core/matrix/csr.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
public ConvertibleTo<Sellp<ValueType, IndexType>>,
public ConvertibleTo<SparsityCsr<ValueType, IndexType>>,
public DiagonalExtractable<ValueType>,
public SubMatrixExtractable<Csr<ValueType, IndexType>>,
public ReadableFromMatrixData<ValueType, IndexType>,
public WritableToMatrixData<ValueType, IndexType>,
public Transposable,
Expand Down Expand Up @@ -765,9 +764,9 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
std::unique_ptr<Diagonal<ValueType>> extract_diagonal() const override;

std::unique_ptr<Csr<ValueType, IndexType>> 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<absolute_type> compute_absolute() const override;
std::unique_ptr<absolute_type> compute_absolute() const;

void compute_absolute_inplace() override;

Expand Down Expand Up @@ -985,30 +984,6 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
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<const Executor> exec, const dim<2>& size,
Array<IndexType>&& row_ptrs,
std::shared_ptr<strategy_type> strategy = std::make_shared<sparselib>())
: EnableLinOp<Csr>(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<ValueType>(exec, num_nnz);
col_idxs_ = Array<IndexType>(exec, num_nnz);
this->make_srow();
}

/**
* Creates a CSR matrix from already allocated (and initialized) row
* pointer, column index and value arrays.
Expand Down
15 changes: 9 additions & 6 deletions omp/test/matrix/csr_kernels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -746,23 +746,26 @@ 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<int>(this->omp, row_nnz);
auto smat1 =
Mtx::create(this->ref, gko::dim<2>(rspan.length(), cspan.length()),
std::move(gko::Array<ValueType>(this->ref, num_nnz)),
std::move(gko::Array<IndexType>(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<ValueType>(this->omp, num_nnz)),
std::move(gko::Array<IndexType>(this->omp, num_nnz)),
std::move(drow_nnz));


gko::kernels::reference::csr::compute_submatrix(this->ref, this->mtx2.get(),
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);
}


Expand Down

0 comments on commit 830d4a6

Please sign in to comment.