review updates:

- documentation
- missing tests
- refactor

Co-authored-by: Thomas Grützmacher <[email protected]>
Co-authored-by: nvenko <[email protected]>
Signed-off-by: Marcel Koch <[email protected]>

Author: 3 people

common/unified/solver/minres_kernels.cpp

@@ -21,10 +21,10 @@ namespace minres {
 namespace detail {
 
 
-template <typename T, typename U>
-GKO_INLINE GKO_ATTRIBUTES void swap(T& a, U& b)
+template <typename T>
+GKO_INLINE GKO_ATTRIBUTES void swap(T& a, T& b)
 {
-    U tmp{b};
+    T tmp{b};
     b = a;
     a = tmp;
 }

core/solver/minres.cpp

@@ -2,6 +2,8 @@
 //
 // SPDX-License-Identifier: BSD-3-Clause
 
+#include <utility>
+
 #include <ginkgo/core/base/exception.hpp>
 #include <ginkgo/core/base/exception_helpers.hpp>
 #include <ginkgo/core/base/executor.hpp>
@@ -25,6 +27,7 @@ GKO_REGISTER_OPERATION(initialize, minres::initialize);
 GKO_REGISTER_OPERATION(step_1, minres::step_1);
 GKO_REGISTER_OPERATION(step_2, minres::step_2);
 
+
 }  // anonymous namespace
 }  // namespace minres
 

core/solver/minres_kernels.hpp

@@ -19,7 +19,7 @@
 
 namespace gko {
 namespace kernels {
-namespace cg {
+namespace minres {
 
 
 #define GKO_DECLARE_MINRES_INITIALIZE_KERNEL(_type)                            \
@@ -67,7 +67,7 @@ namespace cg {
     GKO_DECLARE_MINRES_STEP_2_KERNEL(ValueType)
 
 
-}  // namespace cg
+}  // namespace minres
 
 
 GKO_DECLARE_FOR_ALL_EXECUTOR_NAMESPACES(minres, GKO_DECLARE_ALL_AS_TEMPLATES);

include/ginkgo/core/solver/minres.hpp

@@ -35,6 +35,11 @@ namespace solver {
  * use of data locality. The inner operations in one iteration of Minres are
  * merged into 2 separate steps.
  *
+ * For more details see Anne Grennbaum's 'Iterative Methods
+ * for Solving Linear Systems' (DOI: 10.1137/1.9781611970937), and Sou-Cheng
+ * (Terrya) Choi's 'ITERATIVE METHODS FOR SINGULAR LINEAR EQUATIONS AND
+ * LEAST-SQUARES PROBLEMS'.
+ *
  * @tparam ValueType  precision of matrix elements
  *
  * @ingroup solvers

reference/solver/minres_kernels.cpp

@@ -4,6 +4,8 @@
 
 #include "core/solver/minres_kernels.hpp"
 
+#include <utility>
+
 #include <ginkgo/core/base/array.hpp>
 #include <ginkgo/core/base/exception_helpers.hpp>
 #include <ginkgo/core/base/math.hpp>

reference/test/solver/minres_kernels.cpp

@@ -18,8 +18,10 @@
 
 #include "core/test/utils.hpp"
 
+
 namespace {
 
+
 template <typename T>
 class Minres : public ::testing::Test {
 protected:
@@ -178,6 +180,32 @@ TYPED_TEST(Minres, KernelInitialize)
 }
 
 
+TYPED_TEST(Minres, KernelInitializeWithSafeDivide)
+{
+    using Mtx = typename TestFixture::Mtx;
+    using vt = typename TestFixture::value_type;
+    this->small_r =
+        gko::initialize<Mtx>(I<I<vt>>({{1, 2}, {3, 4}}), this->exec);
+    this->small_z = gko::initialize<Mtx>(I<I<vt>>{{4, 3}, {2, 1}}, this->exec);
+    auto zero = gko::zero<vt>();
+    this->beta = gko::initialize<Mtx>(I<I<vt>>{{zero, zero}}, this->exec);
+    this->small_q->fill(1);
+
+    gko::kernels::reference::minres::initialize(
+        this->exec, this->small_r.get(), this->small_z.get(),
+        this->small_p.get(), this->small_p_prev.get(), this->small_q.get(),
+        this->small_q_prev.get(), this->small_v.get(), this->beta.get(),
+        this->gamma.get(), this->delta.get(), this->cos_prev.get(),
+        this->cos.get(), this->sin_prev.get(), this->sin.get(),
+        this->eta_next.get(), this->eta.get(), &this->small_stop);
+
+    GKO_ASSERT_MTX_NEAR(this->small_q, l({{0.0, 2. / 5}, {0.0, 4. / 5}}),
+                        r<vt>::value);
+    GKO_ASSERT_MTX_NEAR(this->small_z, l({{0.0, 3. / 5}, {0.0, 1. / 5}}),
+                        r<vt>::value);
+}
+
+
 TYPED_TEST(Minres, KernelStep1)
 {
     using Mtx = typename TestFixture::Mtx;
@@ -273,6 +301,36 @@ TYPED_TEST(Minres, KernelStep2)
 }
 
 
+TYPED_TEST(Minres, KernelStep2WithSafeDivide)
+{
+    using Mtx = typename TestFixture::Mtx;
+    using vt = typename TestFixture::value_type;
+    this->small_q = gko::initialize<Mtx>(I<I<vt>>{{4, 9}, {7, 11}}, this->exec);
+    this->small_p = gko::initialize<Mtx>(I<I<vt>>{{1, 2}, {3, 4}}, this->exec);
+    this->small_z = gko::initialize<Mtx>(I<I<vt>>{{6, 1}, {7, 3}}, this->exec);
+    auto zero = gko::zero<vt>();
+    this->alpha = gko::initialize<Mtx>(I<I<vt>>{{zero, zero}}, this->exec);
+    this->beta = gko::initialize<Mtx>(I<I<vt>>{{zero, zero}}, this->exec);
+    auto old_small_q = gko::clone(this->small_q);
+    auto old_small_v = gko::clone(this->small_v);
+    auto old_small_q_scaled = gko::clone(this->small_v);
+    auto old_small_z_tilde_scaled = gko::clone(this->small_z_tilde);
+    auto old_small_v_scaled = gko::clone(this->small_q);
+
+    gko::kernels::reference::minres::step_2(
+        this->exec, this->small_x.get(), this->small_p.get(),
+        this->small_p_prev.get(), this->small_z.get(),
+        this->small_z_tilde.get(), this->small_q.get(),
+        this->small_q_prev.get(), this->small_v.get(), this->alpha.get(),
+        this->beta.get(), this->gamma.get(), this->delta.get(), this->cos.get(),
+        this->eta.get(), &this->small_stop);
+
+    GKO_ASSERT_MTX_NEAR(this->small_q, l({{0.0, 0.0}, {0.0, 0.0}}), 0.);
+    GKO_ASSERT_MTX_NEAR(this->small_z, l({{0.0, 0.0}, {0.0, 0.0}}), 0.);
+    GKO_ASSERT_MTX_NEAR(this->small_p, l({{0.0, 0.0}, {0.0, 0.0}}), 0.);
+}
+
+
 TYPED_TEST(Minres, SolvesSystem)
 {
     using Mtx = typename TestFixture::Mtx;

test/solver/minres_kernels.cpp

@@ -22,31 +22,11 @@
 
 namespace {
 
-class Minres : public ::testing::Test {
+class Minres : public CommonTestFixture {
 protected:
-#if GINKGO_COMMON_SINGLE_MODE
-    using value_type = float;
-#else
-    using value_type = double;
-#endif
     using Mtx = gko::matrix::Dense<value_type>;
     using Solver = gko::solver::Minres<value_type>;
 
-    Minres() : rand_engine(42) {}
-
-    void SetUp()
-    {
-        ref = gko::ReferenceExecutor::create();
-        init_executor(ref, exec);
-    }
-
-    void TearDown()
-    {
-        if (exec != nullptr) {
-            ASSERT_NO_THROW(exec->synchronize());
-        }
-    }
-
     std::unique_ptr<Mtx> gen_mtx(gko::size_type num_rows,
                                  gko::size_type num_cols, gko::size_type stride,
                                  bool make_hermitian)
@@ -126,11 +106,7 @@ class Minres : public ::testing::Test {
             exec, *stop_status);
     }
 
-
-    std::shared_ptr<gko::ReferenceExecutor> ref;
-    std::shared_ptr<gko::EXEC_TYPE> exec;
-
-    std::default_random_engine rand_engine;
+    std::default_random_engine rand_engine{42};
 
     std::unique_ptr<Mtx> x;
     std::unique_ptr<Mtx> b;