Improve CSR strategies

- All conversions to CSR (except CSR to CSR) now conserve the original strategy (test for that are included) - CSR strategies now try to copy the least amount of data - Add exception throwing in case the load_balance would not work properly (when srow is empty)
ginkgo-project · tcojean · Dec 2, 2019 · Oct 21, 2019 · Oct 29, 2019 · Oct 30, 2019
commit 4db298cd70828f3e736856827dfab78007377247
diff --git a/core/matrix/coo.cpp b/core/matrix/coo.cpp
@@ -108,10 +108,13 @@ void Coo<ValueType, IndexType>::convert_to(
     Csr<ValueType, IndexType> *result) const
 {
     auto exec = this->get_executor();
-    auto tmp = Csr<ValueType, IndexType>::create(exec, this->get_size());
+    auto tmp = Csr<ValueType, IndexType>::create(
+        exec, this->get_size(), this->get_num_stored_elements(),
+        result->get_strategy());
     tmp->values_ = this->values_;
     tmp->col_idxs_ = this->col_idxs_;
     exec->run(coo::make_convert_to_csr(tmp.get(), this));
+    tmp->make_srow();
     tmp->move_to(result);
 }
 
@@ -120,10 +123,13 @@ template <typename ValueType, typename IndexType>
 void Coo<ValueType, IndexType>::move_to(Csr<ValueType, IndexType> *result)
 {
     auto exec = this->get_executor();
-    auto tmp = Csr<ValueType, IndexType>::create(exec, this->get_size());
+    auto tmp = Csr<ValueType, IndexType>::create(
+        exec, this->get_size(), this->get_num_stored_elements(),
+        result->get_strategy());
     tmp->values_ = std::move(this->values_);
     tmp->col_idxs_ = std::move(this->col_idxs_);
     exec->run(coo::make_convert_to_csr(tmp.get(), this));
+    tmp->make_srow();
     tmp->move_to(result);
 }
 

diff --git a/core/matrix/ell.cpp b/core/matrix/ell.cpp
@@ -134,10 +134,11 @@ void Ell<ValueType, IndexType>::convert_to(
     size_type num_stored_elements = 0;
     exec->run(ell::make_count_nonzeros(this, &num_stored_elements));
 
-    auto tmp = Csr<ValueType, IndexType>::create(exec, this->get_size(),
-                                                 num_stored_elements);
+    auto tmp = Csr<ValueType, IndexType>::create(
+        exec, this->get_size(), num_stored_elements, result->get_strategy());
     exec->run(ell::make_convert_to_csr(tmp.get(), this));
 
+    tmp->make_srow();
     tmp->move_to(result);
 }
 

diff --git a/core/matrix/hybrid.cpp b/core/matrix/hybrid.cpp
@@ -135,10 +135,11 @@ void Hybrid<ValueType, IndexType>::convert_to(
     size_type num_stored_elements = 0;
     exec->run(hybrid::make_count_nonzeros(this, &num_stored_elements));
 
-    auto tmp = Csr<ValueType, IndexType>::create(exec, this->get_size(),
-                                                 num_stored_elements);
+    auto tmp = Csr<ValueType, IndexType>::create(
+        exec, this->get_size(), num_stored_elements, result->get_strategy());
     exec->run(hybrid::make_convert_to_csr(tmp.get(), this));
 
+    tmp->make_srow();
     tmp->move_to(result);
 }
 

diff --git a/core/matrix/sellp.cpp b/core/matrix/sellp.cpp
@@ -147,9 +147,10 @@ void Sellp<ValueType, IndexType>::convert_to(
 
     size_type num_stored_nonzeros = 0;
     exec->run(sellp::make_count_nonzeros(this, &num_stored_nonzeros));
-    auto tmp = Csr<ValueType, IndexType>::create(exec, this->get_size(),
-                                                 num_stored_nonzeros);
+    auto tmp = Csr<ValueType, IndexType>::create(
+        exec, this->get_size(), num_stored_nonzeros, result->get_strategy());
     exec->run(sellp::make_convert_to_csr(tmp.get(), this));
+    tmp->make_srow();
     tmp->move_to(result);
 }
 

diff --git a/core/test/matrix/csr.cpp b/core/test/matrix/csr.cpp
@@ -127,11 +127,13 @@ TEST_F(Csr, CanBeCreatedFromExistingData)
     double values[] = {1.0, 2.0, 3.0, 4.0};
     gko::int32 col_idxs[] = {0, 1, 1, 0};
     gko::int32 row_ptrs[] = {0, 2, 3, 4};
+
     auto mtx = gko::matrix::Csr<>::create(
         exec, gko::dim<2>{3, 2}, gko::Array<double>::view(exec, 4, values),
         gko::Array<gko::int32>::view(exec, 4, col_idxs),
         gko::Array<gko::int32>::view(exec, 4, row_ptrs),
         std::make_shared<Mtx::load_balance>(2));
+
     ASSERT_EQ(mtx->get_num_srow_elements(), 1);
     ASSERT_EQ(mtx->get_const_values(), values);
     ASSERT_EQ(mtx->get_const_col_idxs(), col_idxs);
@@ -183,6 +185,7 @@ TEST_F(Csr, CanBeCleared)
 TEST_F(Csr, CanBeReadFromMatrixData)
 {
     auto m = Mtx::create(exec, std::make_shared<Mtx::load_balance>(2));
+
     m->read({{2, 3},
              {{0, 0, 1.0},
               {0, 1, 3.0},

diff --git a/include/ginkgo/core/matrix/csr.hpp b/include/ginkgo/core/matrix/csr.hpp
@@ -183,13 +183,28 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
             auto nwarps = mtx_srow->get_num_elems();
 
             if (nwarps > 0) {
-                auto exec = mtx_srow->get_executor()->get_master();
-                Array<index_type> srow_host(exec);
-                srow_host = *mtx_srow;
-                auto srow = srow_host.get_data();
-                Array<index_type> row_ptrs_host(exec);
-                row_ptrs_host = mtx_row_ptrs;
-                auto row_ptrs = row_ptrs_host.get_const_data();
+                auto host_srow_exec = mtx_srow->get_executor()->get_master();
+                auto host_mtx_exec = mtx_row_ptrs.get_executor()->get_master();
+                bool is_srow_on_host{host_srow_exec ==
+                                     mtx_srow->get_executor()};
+                bool is_mtx_on_host{host_mtx_exec ==
+                                    mtx_row_ptrs.get_executor()};
+                Array<index_type> row_ptrs_host(host_mtx_exec);
+                Array<index_type> srow_host(host_srow_exec);
+                const index_type *row_ptrs{};
+                index_type *srow{};
+                if (is_srow_on_host) {
+                    srow = mtx_srow->get_data();
+                } else {
+                    srow_host = *mtx_srow;
+                    srow = srow_host.get_data();
+                }
+                if (is_mtx_on_host) {
+                    row_ptrs = mtx_row_ptrs.get_const_data();
+                } else {
+                    row_ptrs_host = mtx_row_ptrs;
+                    row_ptrs = row_ptrs_host.get_const_data();
+                }
                 for (size_type i = 0; i < nwarps; i++) {
                     srow[i] = 0;
                 }
@@ -207,7 +222,11 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
                 for (size_type i = 1; i < nwarps; i++) {
                     srow[i] += srow[i - 1];
                 }
-                *mtx_srow = srow_host;
+                if (!is_srow_on_host) {
+                    *mtx_srow = srow_host;
+                }
+            } else {
+                GKO_NOT_SUPPORTED(nwarps);
             }
         }
 
@@ -249,28 +268,46 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
             // if the number of stored elements is larger than 1e6 or
             // the maximum number of stored elements per row is larger than
             // 64, use load_balance otherwise use classical
+            auto host_mtx_exec = mtx_row_ptrs.get_executor()->get_master();
+            bool is_mtx_on_host{host_mtx_exec == mtx_row_ptrs.get_executor()};
+            Array<index_type> row_ptrs_host(host_mtx_exec);
+            const index_type *row_ptrs{};
+            if (is_mtx_on_host) {
+                row_ptrs = mtx_row_ptrs.get_const_data();
+            } else {
+                row_ptrs_host = mtx_row_ptrs;
+                row_ptrs = row_ptrs_host.get_const_data();
+            }
             const auto num_rows = mtx_row_ptrs.get_num_elems() - 1;
-            Array<index_type> host_row_ptrs(
-                mtx_row_ptrs.get_executor()->get_master());
-            host_row_ptrs = mtx_row_ptrs;
-            const auto row_val = host_row_ptrs.get_const_data();
-            if (row_val[num_rows] > static_cast<index_type>(1e6)) {
-                std::make_shared<load_balance>(nwarps_)->process(host_row_ptrs,
-                                                                 mtx_srow);
-                this->set_name("load_balance");
+            if (row_ptrs[num_rows] > static_cast<index_type>(1e6)) {
+                load_balance actual_strategy(nwarps_);
+                if (is_mtx_on_host) {
+                    actual_strategy.process(mtx_row_ptrs, mtx_srow);
+                } else {
+                    actual_strategy.process(row_ptrs_host, mtx_srow);
+                }
+                this->set_name(actual_strategy.get_name());
             } else {
                 index_type maxnum = 0;
                 for (index_type i = 1; i < num_rows + 1; i++) {
-                    maxnum = max(maxnum, row_val[i] - row_val[i - 1]);
+                    maxnum = max(maxnum, row_ptrs[i] - row_ptrs[i - 1]);
                 }
                 if (maxnum > 64) {
-                    std::make_shared<load_balance>(nwarps_)->process(
-                        host_row_ptrs, mtx_srow);
-                    this->set_name("load_balance");
+                    load_balance actual_strategy(nwarps_);
+                    if (is_mtx_on_host) {
+                        actual_strategy.process(mtx_row_ptrs, mtx_srow);
+                    } else {
+                        actual_strategy.process(row_ptrs_host, mtx_srow);
+                    }
+                    this->set_name(actual_strategy.get_name());
                 } else {
-                    std::make_shared<classical>()->process(host_row_ptrs,
-                                                           mtx_srow);
-                    this->set_name("classical");
+                    classical actual_strategy;
+                    if (is_mtx_on_host) {
+                        actual_strategy.process(mtx_row_ptrs, mtx_srow);
+                    } else {
+                        actual_strategy.process(row_ptrs_host, mtx_srow);
+                    }
+                    this->set_name(actual_strategy.get_name());
                 }
             }
         }
@@ -501,7 +538,6 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
         GKO_ENSURE_IN_BOUNDS(values_.get_num_elems() - 1,
                              col_idxs_.get_num_elems());
         GKO_ENSURE_IN_BOUNDS(this->get_size()[0], row_ptrs_.get_num_elems());
-        srow_.resize_and_reset(strategy_->clac_size(values_.get_num_elems()));
         this->make_srow();
     }
 
@@ -511,9 +547,13 @@ class Csr : public EnableLinOp<Csr<ValueType, IndexType>>,
                     LinOp *x) const override;
 
     /**
-     * Compute srow, it should be run after setting value.
+     * Computes srow. It should be run after changing any row_ptrs_ value.
      */
-    void make_srow() { strategy_->process(row_ptrs_, &srow_); }
+    void make_srow()
+    {
+        srow_.resize_and_reset(strategy_->clac_size(values_.get_num_elems()));
+        strategy_->process(row_ptrs_, &srow_);
+    }
 
 private:
     Array<value_type> values_;

diff --git a/reference/test/matrix/coo_kernels.cpp b/reference/test/matrix/coo_kernels.cpp
@@ -33,6 +33,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "core/matrix/coo_kernels.hpp"
 
 
+#include <memory>
+
+
 #include <gtest/gtest.h>
 
 
@@ -90,21 +93,40 @@ class Coo : public ::testing::Test {
 
 TEST_F(Coo, ConvertsToCsr)
 {
-    auto csr_mtx = gko::matrix::Csr<>::create(mtx->get_executor());
-
-    mtx->convert_to(csr_mtx.get());
-
-    assert_equal_to_mtx_in_csr_format(csr_mtx.get());
+    auto csr_s_classical = std::make_shared<gko::matrix::Csr<>::classical>();
+    auto csr_s_merge = std::make_shared<gko::matrix::Csr<>::merge_path>();
+    auto csr_mtx_c =
+        gko::matrix::Csr<>::create(mtx->get_executor(), csr_s_classical);
+    auto csr_mtx_m =
+        gko::matrix::Csr<>::create(mtx->get_executor(), csr_s_merge);
+
+    mtx->convert_to(csr_mtx_c.get());
+    mtx->convert_to(csr_mtx_m.get());
+
+    assert_equal_to_mtx_in_csr_format(csr_mtx_c.get());
+    assert_equal_to_mtx_in_csr_format(csr_mtx_m.get());
+    ASSERT_EQ(csr_mtx_c->get_strategy(), csr_s_classical);
+    ASSERT_EQ(csr_mtx_m->get_strategy(), csr_s_merge);
 }
 
 
 TEST_F(Coo, MovesToCsr)
 {
-    auto csr_mtx = gko::matrix::Csr<>::create(mtx->get_executor());
-
-    mtx->move_to(csr_mtx.get());
-
-    assert_equal_to_mtx_in_csr_format(csr_mtx.get());
+    auto csr_s_classical = std::make_shared<gko::matrix::Csr<>::classical>();
+    auto csr_s_merge = std::make_shared<gko::matrix::Csr<>::merge_path>();
+    auto csr_mtx_c =
+        gko::matrix::Csr<>::create(mtx->get_executor(), csr_s_classical);
+    auto csr_mtx_m =
+        gko::matrix::Csr<>::create(mtx->get_executor(), csr_s_merge);
+    auto mtx_clone = mtx->clone();
+
+    mtx->move_to(csr_mtx_c.get());
+    mtx_clone->move_to(csr_mtx_m.get());
+
+    assert_equal_to_mtx_in_csr_format(csr_mtx_c.get());
+    assert_equal_to_mtx_in_csr_format(csr_mtx_m.get());
+    ASSERT_EQ(csr_mtx_c->get_strategy(), csr_s_classical);
+    ASSERT_EQ(csr_mtx_m->get_strategy(), csr_s_merge);
 }
 
 

diff --git a/reference/test/matrix/dense_kernels.cpp b/reference/test/matrix/dense_kernels.cpp
@@ -34,6 +34,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
 #include <complex>
+#include <memory>
 #include <random>
 
 
@@ -333,15 +334,21 @@ TEST_F(Dense, MovesToCoo)
 
 TEST_F(Dense, ConvertsToCsr)
 {
-    auto csr_mtx = gko::matrix::Csr<>::create(mtx4->get_executor());
-
-    mtx4->convert_to(csr_mtx.get());
-    auto v = csr_mtx->get_const_values();
-    auto c = csr_mtx->get_const_col_idxs();
-    auto r = csr_mtx->get_const_row_ptrs();
-
-    ASSERT_EQ(csr_mtx->get_size(), gko::dim<2>(2, 3));
-    ASSERT_EQ(csr_mtx->get_num_stored_elements(), 4);
+    auto csr_s_classical = std::make_shared<gko::matrix::Csr<>::classical>();
+    auto csr_s_merge = std::make_shared<gko::matrix::Csr<>::merge_path>();
+    auto csr_mtx_c =
+        gko::matrix::Csr<>::create(mtx4->get_executor(), csr_s_classical);
+    auto csr_mtx_m =
+        gko::matrix::Csr<>::create(mtx4->get_executor(), csr_s_merge);
+
+    mtx4->convert_to(csr_mtx_c.get());
+    mtx4->convert_to(csr_mtx_m.get());
+
+    auto v = csr_mtx_c->get_const_values();
+    auto c = csr_mtx_c->get_const_col_idxs();
+    auto r = csr_mtx_c->get_const_row_ptrs();
+    ASSERT_EQ(csr_mtx_c->get_size(), gko::dim<2>(2, 3));
+    ASSERT_EQ(csr_mtx_c->get_num_stored_elements(), 4);
     EXPECT_EQ(r[0], 0);
     EXPECT_EQ(r[1], 3);
     EXPECT_EQ(r[2], 4);
@@ -353,20 +360,30 @@ TEST_F(Dense, ConvertsToCsr)
     EXPECT_EQ(v[1], 3.0);
     EXPECT_EQ(v[2], 2.0);
     EXPECT_EQ(v[3], 5.0);
+    ASSERT_EQ(csr_mtx_c->get_strategy(), csr_s_classical);
+    GKO_ASSERT_MTX_NEAR(csr_mtx_c.get(), csr_mtx_m.get(), 0.0);
+    ASSERT_EQ(csr_mtx_m->get_strategy(), csr_s_merge);
 }
 
 
 TEST_F(Dense, MovesToCsr)
 {
-    auto csr_mtx = gko::matrix::Csr<>::create(mtx4->get_executor());
-
-    mtx4->move_to(csr_mtx.get());
-    auto v = csr_mtx->get_const_values();
-    auto c = csr_mtx->get_const_col_idxs();
-    auto r = csr_mtx->get_const_row_ptrs();
-
-    ASSERT_EQ(csr_mtx->get_size(), gko::dim<2>(2, 3));
-    ASSERT_EQ(csr_mtx->get_num_stored_elements(), 4);
+    auto csr_s_classical = std::make_shared<gko::matrix::Csr<>::classical>();
+    auto csr_s_merge = std::make_shared<gko::matrix::Csr<>::merge_path>();
+    auto csr_mtx_c =
+        gko::matrix::Csr<>::create(mtx4->get_executor(), csr_s_classical);
+    auto csr_mtx_m =
+        gko::matrix::Csr<>::create(mtx4->get_executor(), csr_s_merge);
+    auto mtx_clone = mtx4->clone();
+
+    mtx4->move_to(csr_mtx_c.get());
+    mtx_clone->move_to(csr_mtx_m.get());
+
+    auto v = csr_mtx_c->get_const_values();
+    auto c = csr_mtx_c->get_const_col_idxs();
+    auto r = csr_mtx_c->get_const_row_ptrs();
+    ASSERT_EQ(csr_mtx_c->get_size(), gko::dim<2>(2, 3));
+    ASSERT_EQ(csr_mtx_c->get_num_stored_elements(), 4);
     EXPECT_EQ(r[0], 0);
     EXPECT_EQ(r[1], 3);
     EXPECT_EQ(r[2], 4);
@@ -378,6 +395,9 @@ TEST_F(Dense, MovesToCsr)
     EXPECT_EQ(v[1], 3.0);
     EXPECT_EQ(v[2], 2.0);
     EXPECT_EQ(v[3], 5.0);
+    ASSERT_EQ(csr_mtx_c->get_strategy(), csr_s_classical);
+    GKO_ASSERT_MTX_NEAR(csr_mtx_c.get(), csr_mtx_m.get(), 0.0);
+    ASSERT_EQ(csr_mtx_m->get_strategy(), csr_s_merge);
 }