From 8e39367f464800645026e5ab0a6a6a92bf78fb39 Mon Sep 17 00:00:00 2001 From: William Throwe Date: Tue, 12 Mar 2024 14:56:35 -0400 Subject: [PATCH] Don't dense-output to the end of the step This will be necessary for monotonic Adams-Moulton, as the solution is discontinuous there. It should be fine for the others, although it might be nice if we passed more information so the TimeStepper could make the choice. --- .../Actions/RunEventsAndDenseTriggers.hpp | 5 +++-- src/Time/TimeSteppers/AdamsMoultonPc.cpp | 4 ---- src/Time/TimeSteppers/Rk3HesthavenSsp.cpp | 5 ----- src/Time/TimeSteppers/RungeKutta.cpp | 5 ----- .../Actions/Test_RunEventsAndDenseTriggers.cpp | 14 ++++++++++++++ .../Time/TimeSteppers/TimeStepperTestUtils.cpp | 4 ++-- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/Evolution/Actions/RunEventsAndDenseTriggers.hpp b/src/Evolution/Actions/RunEventsAndDenseTriggers.hpp index bdeec2eb5c4f..a0175467b29c 100644 --- a/src/Evolution/Actions/RunEventsAndDenseTriggers.hpp +++ b/src/Evolution/Actions/RunEventsAndDenseTriggers.hpp @@ -204,7 +204,8 @@ struct RunEventsAndDenseTriggers { const auto step_end = time_step_id.step_time() + db::get<::Tags::TimeStep>(box); - const evolution_less before{time_step_id.time_runs_forward()}; + const evolution_less_equal before_equal{ + time_step_id.time_runs_forward()}; using postprocessor_return_tags = tmpl::join>>; @@ -223,7 +224,7 @@ struct RunEventsAndDenseTriggers { for (;;) { const double next_trigger = events_and_dense_triggers.next_trigger(box); - if (before(step_end.value(), next_trigger)) { + if (before_equal(step_end.value(), next_trigger)) { return {Parallel::AlgorithmExecution::Continue, std::nullopt}; } diff --git a/src/Time/TimeSteppers/AdamsMoultonPc.cpp b/src/Time/TimeSteppers/AdamsMoultonPc.cpp index ab14b2057f52..6d880ab68936 100644 --- a/src/Time/TimeSteppers/AdamsMoultonPc.cpp +++ b/src/Time/TimeSteppers/AdamsMoultonPc.cpp @@ -184,10 +184,6 @@ template bool AdamsMoultonPc::dense_update_u_impl(const gsl::not_null u, const ConstUntypedHistory& history, const double time) const { - // Special case required to handle the initial time. - if (time == history.back().time_step_id.step_time().value()) { - return true; - } if (history.at_step_start()) { return false; } diff --git a/src/Time/TimeSteppers/Rk3HesthavenSsp.cpp b/src/Time/TimeSteppers/Rk3HesthavenSsp.cpp index 9f746b788308..bad89b02e5e7 100644 --- a/src/Time/TimeSteppers/Rk3HesthavenSsp.cpp +++ b/src/Time/TimeSteppers/Rk3HesthavenSsp.cpp @@ -113,11 +113,6 @@ bool Rk3HesthavenSsp::dense_update_u_impl(const gsl::not_null u, } const double step_start = history.front().time_step_id.step_time().value(); const double step_end = history.back().time_step_id.step_time().value(); - if (history.size() == 1 and time == step_end) { - // Special case necessary for dense output at the initial time, - // before taking a step. - return true; - } const evolution_less before{step_end > step_start}; if (history.size() == 1 or before(step_end, time)) { return false; diff --git a/src/Time/TimeSteppers/RungeKutta.cpp b/src/Time/TimeSteppers/RungeKutta.cpp index 2658bc53f1dd..1112068a7a75 100644 --- a/src/Time/TimeSteppers/RungeKutta.cpp +++ b/src/Time/TimeSteppers/RungeKutta.cpp @@ -153,11 +153,6 @@ bool RungeKutta::dense_update_u_impl(const gsl::not_null u, } const double step_start = history.front().time_step_id.step_time().value(); const double step_end = history.back().time_step_id.step_time().value(); - if (history.size() == 1 and time == step_end) { - // Special case necessary for dense output at the initial time, - // before taking a step. - return true; - } const evolution_less before{step_end > step_start}; if (history.size() == 1 or before(step_end, time)) { return false; diff --git a/tests/Unit/Evolution/Actions/Test_RunEventsAndDenseTriggers.cpp b/tests/Unit/Evolution/Actions/Test_RunEventsAndDenseTriggers.cpp index 4109b7efc412..5b847986335b 100644 --- a/tests/Unit/Evolution/Actions/Test_RunEventsAndDenseTriggers.cpp +++ b/tests/Unit/Evolution/Actions/Test_RunEventsAndDenseTriggers.cpp @@ -434,6 +434,20 @@ void test(const bool time_runs_forward) { check_not_reached(true); check_not_reached(false); + // Triggers at the end of the step (should not run) + const auto check_step_end = [&set_up_component, &start_time, &step_size]( + const std::optional& is_triggered) { + MockRuntimeSystem runner{ + {std::make_unique(1)}}; + set_up_component( + &runner, {{start_time + step_size, is_triggered, std::nullopt, false}}); + CHECK(run_if_ready(make_not_null(&runner))); + TestEvent::check_calls({}); + }; + check_step_end(std::nullopt); + check_step_end(true); + check_step_end(false); + // Trigger isn't ready { MockRuntimeSystem runner{ diff --git a/tests/Unit/Helpers/Time/TimeSteppers/TimeStepperTestUtils.cpp b/tests/Unit/Helpers/Time/TimeSteppers/TimeStepperTestUtils.cpp index 19ef611b769d..737be7a99dd5 100644 --- a/tests/Unit/Helpers/Time/TimeSteppers/TimeStepperTestUtils.cpp +++ b/tests/Unit/Helpers/Time/TimeSteppers/TimeStepperTestUtils.cpp @@ -361,13 +361,13 @@ void check_dense_output( auto step = step_size; for (;;) { history.insert(time_id, y, y); - if (not before((time_id.step_time() + step).value(), time)) { + if (before(time, (time_id.step_time() + step).value())) { // Make sure the initial value is preserved. y = 2.0 * *history.complete_step_start().value; if (stepper.dense_update_u(make_not_null(&y), history, time)) { return y - *history.complete_step_start().value; } - REQUIRE(before(time_id.step_time().value(), time)); + REQUIRE(not before(time, time_id.step_time().value())); } y = std::numeric_limits::signaling_NaN(); if (use_error_methods) {