Merge Fix the additional residual computation when passing residual c…

…riterion to IR

This PR fixes the additional residual computation when passing residual criterion to IR.

Moreover, it adds the corresponding test to check the number of residual computation and the default.

Related PR: #1307

core/solver/ir.cpp

@@ -240,6 +240,8 @@ void Ir<ValueType>::apply_dense_impl(const VectorType* dense_b,
             if (stop_criterion->update()
                     .num_iterations(iter)
                     .solution(dense_x)
+                    // we have the residual check later
+                    .ignore_residual_check(true)
                     .check(relative_stopping_id, false, &stop_status,
                            &one_changed)) {
                 break;

core/stop/residual_norm.cpp

@@ -193,6 +193,10 @@ bool ResidualNormBase<ValueType>::check_impl(
     const NormVector* dense_tau;
     if (updater.residual_norm_ != nullptr) {
         dense_tau = as<NormVector>(updater.residual_norm_);
+    } else if (updater.ignore_residual_check_) {
+        // If solver already provide the residual norm, we will still store it.
+        // Otherwise, we skip the residual check.
+        return false;
     } else if (updater.residual_ != nullptr) {
         norm_dispatch<ValueType>(
             [&](auto dense_r) { dense_r->compute_norm2(u_dense_tau_); },

core/test/solver/ir.cpp

@@ -40,6 +40,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
 #include <ginkgo/core/base/executor.hpp>
+#include <ginkgo/core/log/profiler_hook.hpp>
 #include <ginkgo/core/matrix/dense.hpp>
 #include <ginkgo/core/stop/combined.hpp>
 #include <ginkgo/core/stop/iteration.hpp>
@@ -74,7 +75,7 @@ class Ir : public ::testing::Test {
           solver(ir_factory->generate(mtx))
     {}
 
-    std::shared_ptr<const gko::Executor> exec;
+    std::shared_ptr<gko::Executor> exec;
     std::shared_ptr<Mtx> mtx;
     std::shared_ptr<typename Solver::Factory> ir_factory;
     std::unique_ptr<gko::LinOp> solver;
@@ -458,4 +459,41 @@ TYPED_TEST(Ir, SmootherBuildWithFactory)
 }
 
 
+struct TestSummaryWriter : gko::log::ProfilerHook::SummaryWriter {
+    void write(const std::vector<gko::log::ProfilerHook::summary_entry>& e,
+               gko::int64 overhead_ns) override
+    {
+        int matched = 0;
+        for (const auto& data : e) {
+            if (data.name == "residual_norm::residual_norm") {
+                matched++;
+                // Contains make_residual_norm 3 times: The last 4-th iteration
+                // exits due to iteration limit.
+                EXPECT_EQ(data.count, 3);
+            }
+        }
+        // ensure matching once
+        EXPECT_EQ(matched, 1);
+    }
+};
+
+
+TYPED_TEST(Ir, RunResidualNormCheckCorrectTimes)
+{
+    using value_type = typename TestFixture::value_type;
+    using Solver = typename TestFixture::Solver;
+    using Mtx = typename TestFixture::Mtx;
+    auto b = gko::initialize<Mtx>({2, -1.0, 1.0}, this->exec);
+    auto x = gko::initialize<Mtx>({0.0, 0.0, 0.0}, this->exec);
+    auto logger = gko::share(gko::log::ProfilerHook::create_summary(
+        std::make_unique<TestSummaryWriter>()));
+    this->exec->add_logger(logger);
+
+    // solver reaches the iteration limit
+    this->solver->apply(b, x);
+
+    // The assertions happen in the destructor of `logger`
+}
+
+
 }  // namespace

core/test/stop/criterion.cpp

@@ -110,6 +110,17 @@ class Criterion : public ::testing::Test {
 };
 
 
+TEST_F(Criterion, DefaultUpdateStatus)
+{
+    EXPECT_EQ(criterion->update().num_iterations_, 0);
+    EXPECT_EQ(criterion->update().ignore_residual_check_, false);
+    EXPECT_EQ(criterion->update().residual_, nullptr);
+    EXPECT_EQ(criterion->update().residual_norm_, nullptr);
+    EXPECT_EQ(criterion->update().implicit_sq_residual_norm_, nullptr);
+    EXPECT_EQ(criterion->update().solution_, nullptr);
+}
+
+
 TEST_F(Criterion, CanLogCheck)
 {
     auto before_logger = *logger;

include/ginkgo/core/stop/criterion.hpp

@@ -68,7 +68,9 @@ class Criterion : public EnableAbstractPolymorphicObject<Criterion> {
      * Criterion's check function. The pattern used is a Builder, except Updater
      * builds a function's arguments before calling the function itself, and
      * does not build an object. This allows calling a Criterion's check in the
-     * form of: stop_criterion->update() .num_iterations(num_iterations)
+     * form of: stop_criterion->update()
+     *   .num_iterations(num_iterations)
+     *   .ignore_residual_check(ignore_residual_check)
      *   .residual_norm(residual_norm)
      *   .implicit_sq_residual_norm(implicit_sq_residual_norm)
      *   .residual(residual)
@@ -123,6 +125,8 @@ class Criterion : public EnableAbstractPolymorphicObject<Criterion> {
     mutable _type* _name##_ {}
 
         GKO_UPDATER_REGISTER_PARAMETER(size_type, num_iterations);
+        // ignore_residual_check default is false
+        GKO_UPDATER_REGISTER_PARAMETER(bool, ignore_residual_check);
         GKO_UPDATER_REGISTER_PTR_PARAMETER(const LinOp, residual);
         GKO_UPDATER_REGISTER_PTR_PARAMETER(const LinOp, residual_norm);
         GKO_UPDATER_REGISTER_PTR_PARAMETER(const LinOp,

reference/test/stop/residual_norm_kernels.cpp

@@ -135,6 +135,26 @@ TYPED_TEST(ResidualNorm, CanCreateCriterionWithNeededInput)
 }
 
 
+TYPED_TEST(ResidualNorm, CanIgorneResidualNorm)
+{
+    using Mtx = typename TestFixture::Mtx;
+    std::shared_ptr<gko::LinOp> scalar =
+        gko::initialize<Mtx>({1.0}, this->exec_);
+    auto criterion =
+        this->rhs_factory_->generate(nullptr, scalar, nullptr, nullptr);
+    constexpr gko::uint8 RelativeStoppingId{1};
+    bool one_changed{};
+    gko::array<gko::stopping_status> stop_status(this->exec_, 1);
+    stop_status.get_data()[0].reset();
+
+    ASSERT_FALSE(criterion->update().ignore_residual_check(true).check(
+        RelativeStoppingId, true, &stop_status, &one_changed));
+    ASSERT_THROW(criterion->update().check(RelativeStoppingId, true,
+                                           &stop_status, &one_changed),
+                 gko::NotSupported);
+}
+
+
 TYPED_TEST(ResidualNorm, WaitsTillResidualGoal)
 {
     using Mtx = typename TestFixture::Mtx;

test/stop/residual_norm_kernels.cpp

@@ -89,6 +89,28 @@ class ResidualNorm : public CommonTestFixture {
 TYPED_TEST_SUITE(ResidualNorm, gko::test::ValueTypes, TypenameNameGenerator);
 
 
+TYPED_TEST(ResidualNorm, CanIgorneResidualNorm)
+{
+    using Mtx = typename TestFixture::Mtx;
+    using NormVector = typename TestFixture::NormVector;
+    auto initial_res = gko::initialize<Mtx>({100.0}, this->exec);
+    std::shared_ptr<gko::LinOp> rhs = gko::initialize<Mtx>({10.0}, this->exec);
+    auto criterion =
+        this->factory->generate(nullptr, rhs, nullptr, initial_res.get());
+    constexpr gko::uint8 RelativeStoppingId{1};
+    bool one_changed{};
+    gko::array<gko::stopping_status> stop_status(this->ref, 1);
+    stop_status.get_data()[0].reset();
+    stop_status.set_executor(this->exec);
+
+    ASSERT_FALSE(criterion->update().ignore_residual_check(true).check(
+        RelativeStoppingId, true, &stop_status, &one_changed));
+    ASSERT_THROW(criterion->update().check(RelativeStoppingId, true,
+                                           &stop_status, &one_changed),
+                 gko::NotSupported);
+}
+
+
 TYPED_TEST(ResidualNorm, WaitsTillResidualGoal)
 {
     using Mtx = typename TestFixture::Mtx;