From 4c1387e8054ca065e7fd55438e4690ecfe9b1cd4 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Fri, 7 Apr 2023 11:38:24 +0200 Subject: [PATCH 1/5] add ResidualCheckNumCheck Co-authored-by: Tobias Ribizel Co-authored-by: Pratik Nayak --- core/test/solver/ir.cpp | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/core/test/solver/ir.cpp b/core/test/solver/ir.cpp index f1c59e63abb..167abf0b1c8 100644 --- a/core/test/solver/ir.cpp +++ b/core/test/solver/ir.cpp @@ -40,6 +40,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include #include #include @@ -52,6 +53,22 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace { +int count_occurrence(const std::string& str, const std::string& substr) +{ + int occurrence = 0; + std::string::size_type pos = 0; + while (pos < str.length()) { + // no overlapped cases + pos = str.find(substr, pos); + if (pos != std::string::npos) { + pos += substr.length(); + occurrence++; + } + } + return occurrence; +} + + template class Ir : public ::testing::Test { protected: @@ -74,7 +91,7 @@ class Ir : public ::testing::Test { solver(ir_factory->generate(mtx)) {} - std::shared_ptr exec; + std::shared_ptr exec; std::shared_ptr mtx; std::shared_ptr ir_factory; std::unique_ptr solver; @@ -458,4 +475,25 @@ TYPED_TEST(Ir, SmootherBuildWithFactory) } +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({2, -1.0, 1.0}, this->exec); + auto x = gko::initialize({0.0, 0.0, 0.0}, this->exec); + std::stringstream out; + auto logger = gko::share(gko::log::Stream::create( + gko::log::Logger::operation_launched_mask, out)); + this->exec->add_logger(logger); + + // solver reaches the iteration limit + this->solver->apply(b, x); + + // Contains make_residual_norm 3 times: check in initialization and two + // iterations. The last iteration exits due to iteration limit. + ASSERT_EQ(count_occurrence(out.str(), "make_residual_norm"), 3); +} + + } // namespace From 8f7134b6fd16dcbfa22f966770c6770e9047c8c5 Mon Sep 17 00:00:00 2001 From: "Yu-Hsiang M. Tsai" Date: Wed, 22 Mar 2023 11:09:07 +0100 Subject: [PATCH 2/5] fix the additional residual computation when passing residual criterion --- core/stop/residual_norm.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/stop/residual_norm.cpp b/core/stop/residual_norm.cpp index be1801090ee..d4f7c691f74 100644 --- a/core/stop/residual_norm.cpp +++ b/core/stop/residual_norm.cpp @@ -198,8 +198,9 @@ bool ResidualNormBase::check_impl( [&](auto dense_r) { dense_r->compute_norm2(u_dense_tau_); }, updater.residual_); dense_tau = u_dense_tau_.get(); - } else if (updater.solution_ != nullptr && system_matrix_ != nullptr && - b_ != nullptr) { + } else if (set_finalized && updater.solution_ != nullptr && + system_matrix_ != nullptr && b_ != nullptr) { + // Only compute the residual from solution in the finalized step auto exec = this->get_executor(); norm_dispatch( [&](auto dense_b, auto dense_x) { @@ -209,6 +210,10 @@ bool ResidualNormBase::check_impl( }, b_.get(), updater.solution_); dense_tau = u_dense_tau_.get(); + } else if (!set_finalized) { + // If it is not the finalized step and does not contain residual, we + // skip the residual norm check + return false; } else { GKO_NOT_SUPPORTED(nullptr); } From cfeb919828d23001e0db6c45be989156351c2472 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Thu, 23 Mar 2023 00:00:24 +0100 Subject: [PATCH 3/5] additional variable ignore_residual_check --- core/solver/ir.cpp | 2 ++ core/stop/residual_norm.cpp | 13 ++++++------- include/ginkgo/core/stop/criterion.hpp | 5 ++++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/core/solver/ir.cpp b/core/solver/ir.cpp index 5e35e7f916d..47ce1227d09 100644 --- a/core/solver/ir.cpp +++ b/core/solver/ir.cpp @@ -240,6 +240,8 @@ void Ir::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; diff --git a/core/stop/residual_norm.cpp b/core/stop/residual_norm.cpp index d4f7c691f74..ee02c8042d2 100644 --- a/core/stop/residual_norm.cpp +++ b/core/stop/residual_norm.cpp @@ -193,14 +193,17 @@ bool ResidualNormBase::check_impl( const NormVector* dense_tau; if (updater.residual_norm_ != nullptr) { dense_tau = as(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( [&](auto dense_r) { dense_r->compute_norm2(u_dense_tau_); }, updater.residual_); dense_tau = u_dense_tau_.get(); - } else if (set_finalized && updater.solution_ != nullptr && - system_matrix_ != nullptr && b_ != nullptr) { - // Only compute the residual from solution in the finalized step + } else if (updater.solution_ != nullptr && system_matrix_ != nullptr && + b_ != nullptr) { auto exec = this->get_executor(); norm_dispatch( [&](auto dense_b, auto dense_x) { @@ -210,10 +213,6 @@ bool ResidualNormBase::check_impl( }, b_.get(), updater.solution_); dense_tau = u_dense_tau_.get(); - } else if (!set_finalized) { - // If it is not the finalized step and does not contain residual, we - // skip the residual norm check - return false; } else { GKO_NOT_SUPPORTED(nullptr); } diff --git a/include/ginkgo/core/stop/criterion.hpp b/include/ginkgo/core/stop/criterion.hpp index d54544a1fcc..bf8bcd3ad22 100644 --- a/include/ginkgo/core/stop/criterion.hpp +++ b/include/ginkgo/core/stop/criterion.hpp @@ -68,7 +68,9 @@ class Criterion : public EnableAbstractPolymorphicObject { * 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,7 @@ class Criterion : public EnableAbstractPolymorphicObject { mutable _type* _name##_ {} GKO_UPDATER_REGISTER_PARAMETER(size_type, num_iterations); + 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, From 7fd4b3320bc927b04715e5dc62d9f83afcd4d04e Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Fri, 7 Apr 2023 11:37:34 +0200 Subject: [PATCH 4/5] add documentation and test Co-authored-by: Pratik Nayak --- core/test/stop/criterion.cpp | 11 ++++++++++ include/ginkgo/core/stop/criterion.hpp | 1 + reference/test/stop/residual_norm_kernels.cpp | 20 +++++++++++++++++ test/stop/residual_norm_kernels.cpp | 22 +++++++++++++++++++ 4 files changed, 54 insertions(+) diff --git a/core/test/stop/criterion.cpp b/core/test/stop/criterion.cpp index 45205fb2f0e..c00c5aa1851 100644 --- a/core/test/stop/criterion.cpp +++ b/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; diff --git a/include/ginkgo/core/stop/criterion.hpp b/include/ginkgo/core/stop/criterion.hpp index bf8bcd3ad22..e094cc90206 100644 --- a/include/ginkgo/core/stop/criterion.hpp +++ b/include/ginkgo/core/stop/criterion.hpp @@ -125,6 +125,7 @@ class Criterion : public EnableAbstractPolymorphicObject { 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); diff --git a/reference/test/stop/residual_norm_kernels.cpp b/reference/test/stop/residual_norm_kernels.cpp index a0dd7f9744a..cc8d145231e 100644 --- a/reference/test/stop/residual_norm_kernels.cpp +++ b/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 scalar = + gko::initialize({1.0}, this->exec_); + auto criterion = + this->rhs_factory_->generate(nullptr, scalar, nullptr, nullptr); + constexpr gko::uint8 RelativeStoppingId{1}; + bool one_changed{}; + gko::array 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; diff --git a/test/stop/residual_norm_kernels.cpp b/test/stop/residual_norm_kernels.cpp index 8135117d6cb..50b8ae19df1 100644 --- a/test/stop/residual_norm_kernels.cpp +++ b/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({100.0}, this->exec); + std::shared_ptr rhs = gko::initialize({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 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; From d986dbb98848b5bb7a0835f99396238865356dd7 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Fri, 7 Apr 2023 15:14:36 +0200 Subject: [PATCH 5/5] using profilerhook not stream to check number MSVC does not contain enough function information. --- core/test/solver/ir.cpp | 46 ++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/core/test/solver/ir.cpp b/core/test/solver/ir.cpp index 167abf0b1c8..36777876861 100644 --- a/core/test/solver/ir.cpp +++ b/core/test/solver/ir.cpp @@ -40,7 +40,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include -#include +#include #include #include #include @@ -53,22 +53,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace { -int count_occurrence(const std::string& str, const std::string& substr) -{ - int occurrence = 0; - std::string::size_type pos = 0; - while (pos < str.length()) { - // no overlapped cases - pos = str.find(substr, pos); - if (pos != std::string::npos) { - pos += substr.length(); - occurrence++; - } - } - return occurrence; -} - - template class Ir : public ::testing::Test { protected: @@ -475,6 +459,25 @@ TYPED_TEST(Ir, SmootherBuildWithFactory) } +struct TestSummaryWriter : gko::log::ProfilerHook::SummaryWriter { + void write(const std::vector& 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; @@ -482,17 +485,14 @@ TYPED_TEST(Ir, RunResidualNormCheckCorrectTimes) using Mtx = typename TestFixture::Mtx; auto b = gko::initialize({2, -1.0, 1.0}, this->exec); auto x = gko::initialize({0.0, 0.0, 0.0}, this->exec); - std::stringstream out; - auto logger = gko::share(gko::log::Stream::create( - gko::log::Logger::operation_launched_mask, out)); + auto logger = gko::share(gko::log::ProfilerHook::create_summary( + std::make_unique())); this->exec->add_logger(logger); // solver reaches the iteration limit this->solver->apply(b, x); - // Contains make_residual_norm 3 times: check in initialization and two - // iterations. The last iteration exits due to iteration limit. - ASSERT_EQ(count_occurrence(out.str(), "make_residual_norm"), 3); + // The assertions happen in the destructor of `logger` }