Some minor changes and clarifications.

Author: pratikvn

core/solver/trs.cpp

@@ -70,6 +70,11 @@ void Trs<ValueType, IndexType>::generate(const LinOp *system_matrix,
     using CsrMatrix = matrix::Csr<ValueType, IndexType>;
     using Vector = matrix::Dense<ValueType>;
     GKO_ASSERT_IS_SQUARE_MATRIX(system_matrix);
+    // This is needed because it does not make sense to call the copy and
+    // convert if the existing matrix (if not CSR) is empty.
+    if (dynamic_cast<const CsrMatrix *>(system_matrix) == nullptr) {
+        GKO_ASSERT_IS_NON_EMPTY_MATRIX(system_matrix);
+    }
     const auto exec = this->get_executor();
     csr_system_matrix_ = copy_and_convert_to<CsrMatrix>(exec, system_matrix);
     auto dense_b = as<const Vector>(b);

core/test/solver/trs.cpp

@@ -33,8 +33,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <ginkgo/core/solver/trs.hpp>
 
 
-#include <typeinfo>
-
+#include <memory>
 
 #include <gtest/gtest.h>
 
@@ -56,7 +55,6 @@ class Trs : public ::testing::Test {
 
     std::shared_ptr<const gko::Executor> exec;
     std::unique_ptr<Solver::Factory> trs_factory;
-    std::unique_ptr<gko::LinOp> solver;
 };
 
 

dev_tools/scripts/todo_trs.txt

@@ -134,6 +134,22 @@ inline dim<2> get_size(const dim<2> &size) { return size; }
             ::gko::detail::get_size(_op1)[1], "expected square matrix"); \
     }
 
+/**
+ *Asserts that _op1 is a non-empty matrix.
+ *
+ *@throw DimensionMismatch  if the number of rows of _op1 is different from the
+ *                          number of columns of _op1.
+ */
+#define GKO_ASSERT_IS_NON_EMPTY_MATRIX(_op1)                                \
+    if (::gko::detail::get_size(_op1)[0] == 0 &&                            \
+        ::gko::detail::get_size(_op1)[1] == 0) {                            \
+        throw ::gko::DimensionMismatch(                                     \
+            __FILE__, __LINE__, __func__, #_op1,                            \
+            ::gko::detail::get_size(_op1)[0],                               \
+            ::gko::detail::get_size(_op1)[1], #_op1,                        \
+            ::gko::detail::get_size(_op1)[0],                               \
+            ::gko::detail::get_size(_op1)[1], "expected non-empty matrix"); \
+    }
 
 /**
  * Asserts that _op1 can be applied to _op2.

include/ginkgo/core/base/exception_helpers.hpp

@@ -227,7 +227,8 @@ class Trs : public EnableLinOp<Trs<ValueType, IndexType>>,
         : parameters_{factory->get_parameters()},
           EnableLinOp<Trs>(factory->get_executor(),
                            transpose(args.system_matrix->get_size())),
-          system_matrix_{std::move(args.system_matrix)}
+          system_matrix_{std::move(args.system_matrix)},
+          b_{std::move(args.b)}
     {
         if (parameters_.preconditioner) {
             preconditioner_ =
@@ -236,12 +237,13 @@ class Trs : public EnableLinOp<Trs<ValueType, IndexType>>,
             preconditioner_ = matrix::Identity<ValueType>::create(
                 this->get_executor(), this->get_size()[0]);
         }
-        this->generate(gko::lend(system_matrix_), gko::lend(args.b));
+        this->generate(gko::lend(system_matrix_), gko::lend(b_));
     }
 
 
 private:
     std::shared_ptr<const LinOp> system_matrix_{};
+    std::shared_ptr<const LinOp> b_{};
     std::shared_ptr<const matrix::Csr<ValueType, IndexType>>
         csr_system_matrix_{};
     std::shared_ptr<const LinOp> preconditioner_{};

include/ginkgo/core/solver/trs.hpp

@@ -56,7 +56,11 @@ template <typename ValueType, typename IndexType>
 void generate(std::shared_ptr<const OmpExecutor> exec,
               const matrix::Csr<ValueType, IndexType> *matrix,
               const matrix::Dense<ValueType> *b)
-{}
+{
+    // This generate kernel is here to allow for a more sophisticated
+    // implementation as for the CUDA executor. This kernel would perform the
+    // "analysis" phase for the triangular matrix.
+}
 
 
 template <typename ValueType, typename IndexType>

omp/solver/trs_kernels.cpp

@@ -38,7 +38,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <ginkgo/core/base/math.hpp>
 #include <ginkgo/core/base/types.hpp>
 #include <ginkgo/core/matrix/csr.hpp>
-#include <iostream>
+
+
 namespace gko {
 namespace kernels {
 namespace reference {
@@ -53,7 +54,11 @@ template <typename ValueType, typename IndexType>
 void generate(std::shared_ptr<const ReferenceExecutor> exec,
               const matrix::Csr<ValueType, IndexType> *matrix,
               const matrix::Dense<ValueType> *b)
-{}
+{
+    // This generate kernel is here to allow for a more sophisticated
+    // implementation as for the CUDA executor. This kernel would perform the
+    // "analysis" phase for the triangular matrix.
+}
 
 GKO_INSTANTIATE_FOR_EACH_VALUE_AND_INDEX_TYPE(GKO_DECLARE_TRS_GENERATE_KERNEL);
 

reference/solver/trs_kernels.cpp

@@ -32,6 +32,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <ginkgo/core/solver/trs.hpp>
 
+#include <memory>
 
 #include <gtest/gtest.h>
 
@@ -41,10 +42,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <ginkgo/core/base/executor.hpp>
 #include <ginkgo/core/matrix/csr.hpp>
 #include <ginkgo/core/matrix/dense.hpp>
-#include <ginkgo/core/stop/combined.hpp>
-#include <ginkgo/core/stop/iteration.hpp>
-#include <ginkgo/core/stop/residual_norm_reduction.hpp>
-#include <ginkgo/core/stop/time.hpp>
 
 
 namespace {
@@ -83,20 +80,6 @@ class Trs : public ::testing::Test {
             }
         }
     }
-
-    static void assert_same_csr_matrices(const CsrMtx *m1, const CsrMtx *m2)
-    {
-        ASSERT_EQ(m1->get_size()[0], m2->get_size()[0]);
-        ASSERT_EQ(m1->get_size()[1], m2->get_size()[1]);
-
-        for (gko::size_type i = 0; i < m1->get_size()[0] + 1; ++i) {
-            EXPECT_EQ(m1->get_const_row_ptrs()[i], m2->get_const_row_ptrs()[i]);
-        }
-        for (gko::size_type i = 0; i < m1->get_num_stored_elements(); ++i) {
-            EXPECT_EQ(m1->get_const_col_idxs()[i], m2->get_const_col_idxs()[i]);
-            EXPECT_EQ(m1->get_const_values()[i], m2->get_const_values()[i]);
-        }
-    }
 };