Skip to content

Commit

Permalink
Merge fix for criterion check logger
Browse files Browse the repository at this point in the history
Fixes an infinite recursion and a warning about partially overriding the on_criterion_check_completed function.

Related PR: #743
  • Loading branch information
upsj committed Apr 14, 2021
2 parents 9642445 + 4853cfd commit 3be4b70
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 3 deletions.
16 changes: 15 additions & 1 deletion core/log/record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ void Record::on_criterion_check_started(

void Record::on_criterion_check_completed(
const stop::Criterion *criterion, const size_type &num_iterations,
const LinOp *residual, const LinOp *residual_norm, const LinOp *solution,
const LinOp *residual, const LinOp *residual_norm,
const LinOp *implicit_residual_norm_sq, const LinOp *solution,
const uint8 &stopping_id, const bool &set_finalized,
const Array<stopping_status> *status, const bool &oneChanged,
const bool &converged) const
Expand All @@ -257,6 +258,19 @@ void Record::on_criterion_check_completed(
}


void Record::on_criterion_check_completed(
const stop::Criterion *criterion, const size_type &num_iterations,
const LinOp *residual, const LinOp *residual_norm, const LinOp *solution,
const uint8 &stopping_id, const bool &set_finalized,
const Array<stopping_status> *status, const bool &oneChanged,
const bool &converged) const
{
this->on_criterion_check_completed(
criterion, num_iterations, residual, residual_norm, nullptr, solution,
stopping_id, set_finalized, status, oneChanged, converged);
}


void Record::on_iteration_complete(const LinOp *solver,
const size_type &num_iterations,
const LinOp *residual, const LinOp *solution,
Expand Down
32 changes: 31 additions & 1 deletion core/test/log/record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ TEST(Record, CatchesCriterionCheckStarted)
}


TEST(Record, CatchesCriterionCheckCompleted)
TEST(Record, CatchesCriterionCheckCompletedOld)
{
auto exec = gko::ReferenceExecutor::create();
auto logger = gko::log::Record::create(
Expand Down Expand Up @@ -494,6 +494,36 @@ TEST(Record, CatchesCriterionCheckCompleted)
}


TEST(Record, CatchesCriterionCheckCompleted)
{
auto exec = gko::ReferenceExecutor::create();
auto logger = gko::log::Record::create(
exec, gko::log::Logger::criterion_check_completed_mask);
auto criterion =
gko::stop::Iteration::build().with_max_iters(3u).on(exec)->generate(
nullptr, nullptr, nullptr);
constexpr gko::uint8 RelativeStoppingId{42};
gko::Array<gko::stopping_status> stop_status(exec, 1);

logger->on<gko::log::Logger::criterion_check_completed>(
criterion.get(), 1, nullptr, nullptr, nullptr, nullptr,
RelativeStoppingId, true, &stop_status, true, true);

stop_status.get_data()->reset();
stop_status.get_data()->stop(RelativeStoppingId);
auto &data = logger->get().criterion_check_completed.back();
ASSERT_NE(data->criterion, nullptr);
ASSERT_EQ(data->stopping_id, RelativeStoppingId);
ASSERT_EQ(data->set_finalized, true);
ASSERT_EQ(data->status->get_const_data()->has_stopped(), true);
ASSERT_EQ(data->status->get_const_data()->get_id(),
stop_status.get_const_data()->get_id());
ASSERT_EQ(data->status->get_const_data()->is_finalized(), true);
ASSERT_EQ(data->oneChanged, true);
ASSERT_EQ(data->converged, true);
}


TEST(Record, CatchesIterations)
{
using Dense = gko::matrix::Dense<>;
Expand Down
2 changes: 1 addition & 1 deletion include/ginkgo/core/log/logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ public: \
const Array<stopping_status> *status, const bool &one_changed,
const bool &all_converged) const
{
this->on_criterion_check_completed(criterion, it, r, x, tau, x,
this->on_criterion_check_completed(criterion, it, r, tau, x,
stopping_id, set_finalized, status,
one_changed, all_converged);
}
Expand Down
8 changes: 8 additions & 0 deletions include/ginkgo/core/log/record.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,14 @@ class Record : public Logger {
const uint8 &stopping_id,
const bool &set_finalized) const override;

void on_criterion_check_completed(
const stop::Criterion *criterion, const size_type &num_iterations,
const LinOp *residual, const LinOp *residual_norm,
const LinOp *implicit_residual_norm_sq, const LinOp *solution,
const uint8 &stopping_id, const bool &set_finalized,
const Array<stopping_status> *status, const bool &one_changed,
const bool &all_converged) const override;

void on_criterion_check_completed(
const stop::Criterion *criterion, const size_type &num_iterations,
const LinOp *residual, const LinOp *residual_norm,
Expand Down

0 comments on commit 3be4b70

Please sign in to comment.