review updates

- rename functions to match configuration keys
- add documentation
- add missing tests

Co-authored-by: Marcel Koch <[email protected]>
Co-authored-by: Yu-Hsiang M. Tsai <[email protected]>

Author: 3 people

core/stop/iteration.cpp

@@ -24,7 +24,7 @@ bool Iteration::check_impl(uint8 stoppingId, bool setFinalized,
 }
 
 
-deferred_factory_parameter<Iteration::Factory> iteration(size_type count)
+deferred_factory_parameter<Iteration::Factory> max_iters(size_type count)
 {
     return Iteration::build().with_max_iters(count);
 }

core/stop/residual_norm.cpp

@@ -311,23 +311,25 @@ class ResidualNormFactory
 };
 
 
-deferred_factory_parameter<CriterionFactory> abs_residual_norm(double tolerance)
+deferred_factory_parameter<CriterionFactory> absolute_residual_norm(
+    double tolerance)
 {
     return residual_norm_factory_parameters{}
         .with_threshold(tolerance)
         .with_baseline(mode::absolute);
 }
 
 
-deferred_factory_parameter<CriterionFactory> rel_residual_norm(double tolerance)
+deferred_factory_parameter<CriterionFactory> relative_residual_norm(
+    double tolerance)
 {
     return residual_norm_factory_parameters{}
         .with_threshold(tolerance)
         .with_baseline(mode::rhs_norm);
 }
 
 
-deferred_factory_parameter<CriterionFactory> residual_norm_reduction(
+deferred_factory_parameter<CriterionFactory> initial_residual_norm(
     double tolerance)
 {
     return residual_norm_factory_parameters{}
@@ -414,7 +416,7 @@ class ImplicitResidualNormFactory
 };
 
 
-deferred_factory_parameter<CriterionFactory> implicit_abs_residual_norm(
+deferred_factory_parameter<CriterionFactory> absolute_implicit_residual_norm(
     double tolerance)
 {
     return implicit_residual_norm_factory_parameters{}
@@ -423,7 +425,7 @@ deferred_factory_parameter<CriterionFactory> implicit_abs_residual_norm(
 }
 
 
-deferred_factory_parameter<CriterionFactory> implicit_rel_residual_norm(
+deferred_factory_parameter<CriterionFactory> relative_implicit_residual_norm(
     double tolerance)
 {
     return implicit_residual_norm_factory_parameters{}
@@ -432,7 +434,7 @@ deferred_factory_parameter<CriterionFactory> implicit_rel_residual_norm(
 }
 
 
-deferred_factory_parameter<CriterionFactory> implicit_residual_norm_reduction(
+deferred_factory_parameter<CriterionFactory> initial_implicit_residual_norm(
     double tolerance)
 {
     return implicit_residual_norm_factory_parameters{}

core/stop/time.cpp

@@ -4,6 +4,8 @@
 
 #include "ginkgo/core/stop/time.hpp"
 
+#include <chrono>
+
 #include <ginkgo/core/base/abstract_factory.hpp>
 
 
@@ -24,17 +26,10 @@ bool Time::check_impl(uint8 stoppingId, bool setFinalized,
 }
 
 
-deferred_factory_parameter<Time::Factory> time_sec(double time)
-{
-    return Time::build().with_time_limit(
-        std::chrono::nanoseconds{static_cast<long>(time * 1e9)});
-}
-
-
-deferred_factory_parameter<Time::Factory> time_ms(double time)
+deferred_factory_parameter<Time::Factory> time_limit(
+    std::chrono::nanoseconds time)
 {
-    return Time::build().with_time_limit(
-        std::chrono::nanoseconds{static_cast<long>(time * 1e6)});
+    return Time::build().with_time_limit(time);
 }
 
 

include/ginkgo/core/stop/iteration.hpp

@@ -60,7 +60,27 @@ class Iteration : public EnablePolymorphicObject<Iteration, Criterion> {
 };
 
 
-deferred_factory_parameter<Iteration::Factory> iteration(size_type count);
+/**
+ * Creates the precursor to an Iteration stopping criterion factory, to be used
+ * in conjunction with `.with_criteria(...)` function calls when building a
+ * solver factory. This stopping criterion will stop the iteration after `count`
+ * iterations of the solver have finished.
+ *
+ * Full usage example: Stop after 100 iterations or when the relative residual
+ * norm is below $10^{-10}$, whichever happens first.
+ * ```cpp
+ * auto factory = gko::solver::Cg<double>::build()
+ *                    .with_criteria(
+ *                        gko::stop::max_iters(100),
+ *                        gko::stop::relative_residual_norm(1e-10))
+ *                    .on(exec);
+ * ```
+ *
+ * @param count  the number of iterations after which to stop
+ * @return a deferred_factory_parameter that can be passed to the
+ *         `with_criteria` function when building a solver.
+ */
+deferred_factory_parameter<Iteration::Factory> max_iters(size_type count);
 
 
 }  // namespace stop

include/ginkgo/core/stop/residual_norm.hpp

@@ -223,22 +223,58 @@ class ImplicitResidualNorm : public ResidualNormBase<ValueType> {
 };
 
 
-deferred_factory_parameter<CriterionFactory> abs_residual_norm(
+/**
+ * Creates the precursor to a ResidualNorm stopping criterion factory, to be
+ * used in conjunction with `.with_criteria(...)` function calls when building a
+ * solver factory. This stopping criterion will stop the iteration after the
+ * residual norm has decreased below the specified value or by the specified
+ * amount.
+ *
+ * Full usage example: Stop after 100 iterations or when the absolute residual
+ * norm is below $10^{-10}$, whichever happens first.
+ * ```cpp
+ * auto factory = gko::solver::Cg<double>::build()
+ *                    .with_criteria(
+ *                        gko::stop::max_iters(100),
+ *                        gko::stop::absolute_residual_norm(1e-10))
+ *                    .on(exec);
+ * ```
+ *
+ * @param tolerance  the value the residual norm needs to be below.
+ *     With residual $r$, initial guess $x_0$, right-hand side $b$, matrix $A$,
+ *     `absolute` means the exact value of the norm $||r||$,
+ *     `relative` means the norm relative to the right-hand side $||r||/||b||$,
+ *     `initial` means the norm relative to the initial residual
+ *     $||r||/||b - A x_0||$.
+ *     An implicit stopping criterion is only available with some solvers, and
+ *     refers to either the energy norm $||r||_A$ in short-recurrence solvers
+ *     like Cg or the euclidian norm $||r||$ in solvers like GMRES.
+ *     Implicit residual norms are cheaper to compute, but may be less precise
+ *     due to accumulating rounding errors.
+ * @return a deferred_factory_parameter that can be passed to the
+ *         `with_criteria` function when building a solver.
+ */
+deferred_factory_parameter<CriterionFactory> absolute_residual_norm(
     double tolerance);
 
-deferred_factory_parameter<CriterionFactory> rel_residual_norm(
+/** @copydoc absolute_residual_norm */
+deferred_factory_parameter<CriterionFactory> relative_residual_norm(
     double tolerance);
 
-deferred_factory_parameter<CriterionFactory> residual_norm_reduction(
+/** @copydoc absolute_residual_norm */
+deferred_factory_parameter<CriterionFactory> initial_residual_norm(
     double tolerance);
 
-deferred_factory_parameter<CriterionFactory> implicit_abs_residual_norm(
+/** @copydoc absolute_residual_norm */
+deferred_factory_parameter<CriterionFactory> absolute_implicit_residual_norm(
     double tolerance);
 
-deferred_factory_parameter<CriterionFactory> implicit_rel_residual_norm(
+/** @copydoc absolute_residual_norm */
+deferred_factory_parameter<CriterionFactory> relative_implicit_residual_norm(
     double tolerance);
 
-deferred_factory_parameter<CriterionFactory> implicit_residual_norm_reduction(
+/** @copydoc absolute_residual_norm */
+deferred_factory_parameter<CriterionFactory> initial_implicit_residual_norm(
     double tolerance);
 
 

include/ginkgo/core/stop/time.hpp

@@ -69,8 +69,31 @@ class Time : public EnablePolymorphicObject<Time, Criterion> {
 };
 
 
-deferred_factory_parameter<Time::Factory> time_sec(double time);
-deferred_factory_parameter<Time::Factory> time_ms(double time);
+/**
+ * Creates the precursor to a Time stopping criterion factory, to be used
+ * in conjunction with `.with_criteria(...)` function calls when building a
+ * solver factory. This stopping criterion will stop the iteration after the
+ * specified amount of time since the start of the solver run has elapsed.
+ *
+ * Full usage example: Stop after 1 second or when the relative residual norm is
+ * below $10^{-10}$, whichever happens first.
+ * ```cpp
+ * auto factory = gko::solver::Cg<double>::build()
+ *                    .with_criteria(
+ *                        gko::stop::time_limit(std::chrono::seconds(1)),
+ *                        gko::stop::relative_residual_norm(1e-10))
+ *                    .on(exec);
+ * ```
+ *
+ * @param duration  the time limit after which to stop the iteration.
+ *                  Thanks to std::chrono's converting constructors, you can
+ *                  specify any time units, and they will be converted to
+ *                  nanoseconds automatically.
+ * @return a deferred_factory_parameter that can be passed to the
+ *         `with_criteria` function when building a solver.
+ */
+deferred_factory_parameter<Time::Factory> time_limit(
+    std::chrono::nanoseconds duration);
 
 
 }  // namespace stop

reference/test/stop/iteration.cpp

@@ -27,6 +27,13 @@ class Iteration : public ::testing::Test {
 };
 
 
+TEST_F(Iteration, NewInterface)
+{
+    auto factory = gko::stop::max_iters(test_iterations).on(exec_);
+    ASSERT_EQ(factory->get_parameters().max_iters, test_iterations);
+}
+
+
 TEST_F(Iteration, WaitsTillIteration)
 {
     bool one_changed{};

reference/test/stop/residual_norm_kernels.cpp

@@ -518,9 +518,9 @@ TYPED_TEST(ResidualNorm, SimplifiedInterface)
     auto initial_guess = gko::initialize<Mtx>({1000.0}, this->exec_);
     std::shared_ptr<gko::LinOp> rhs = gko::initialize<Mtx>({10.0}, this->exec_);
 
-    auto factory_abs = gko::stop::abs_residual_norm(0.5).on(this->exec_);
-    auto factory_rel = gko::stop::rel_residual_norm(0.5).on(this->exec_);
-    auto factory_red = gko::stop::residual_norm_reduction(0.5).on(this->exec_);
+    auto factory_abs = gko::stop::absolute_residual_norm(0.5).on(this->exec_);
+    auto factory_rel = gko::stop::relative_residual_norm(0.5).on(this->exec_);
+    auto factory_red = gko::stop::initial_residual_norm(0.5).on(this->exec_);
 
     auto crit_abs =
         gko::as<gko::stop::ResidualNorm<TypeParam>>(factory_abs->generate(
@@ -1008,11 +1008,11 @@ TYPED_TEST(ImplicitResidualNorm, SimplifiedInterface)
     std::shared_ptr<gko::LinOp> rhs = gko::initialize<Mtx>({10.0}, this->exec_);
 
     auto factory_abs =
-        gko::stop::implicit_abs_residual_norm(0.5).on(this->exec_);
+        gko::stop::absolute_implicit_residual_norm(0.5).on(this->exec_);
     auto factory_rel =
-        gko::stop::implicit_rel_residual_norm(0.5).on(this->exec_);
+        gko::stop::relative_implicit_residual_norm(0.5).on(this->exec_);
     auto factory_red =
-        gko::stop::implicit_residual_norm_reduction(0.5).on(this->exec_);
+        gko::stop::initial_implicit_residual_norm(0.5).on(this->exec_);
 
     auto crit_abs = gko::as<gko::stop::ImplicitResidualNorm<TypeParam>>(
         factory_abs->generate(nullptr, rhs, initial_guess.get(),

reference/test/stop/time.cpp

@@ -54,6 +54,16 @@ TEST_F(Time, CanCreateFactory)
 }
 
 
+TEST_F(Time, CanCreateFactoryNewInterface)
+{
+    auto new_factory =
+        gko::stop::time_limit(std::chrono::milliseconds(test_ms)).on(exec_);
+    ASSERT_NE(new_factory, nullptr);
+    ASSERT_EQ(new_factory->get_parameters().time_limit,
+              std::chrono::milliseconds(test_ms));
+}
+
+
 TEST_F(Time, CanCreateCriterion)
 {
     auto criterion = factory_->generate(nullptr, nullptr, nullptr);

test/mpi/solver/solver.cpp

@@ -79,8 +79,8 @@ struct SimpleSolverTest {
         std::shared_ptr<const gko::Executor> exec)
     {
         return solver_type::build().with_criteria(
-            gko::stop::iteration(iteration_count()),
-            gko::stop::abs_residual_norm(reduction_factor()).on(exec));
+            gko::stop::max_iters(iteration_count()),
+            gko::stop::absolute_residual_norm(reduction_factor()).on(exec));
     }
 
     static void assert_empty_state(const solver_type* mtx)