Various minor time-related test cleanup

docs/DevGuide/WritingTests.md

@@ -234,7 +234,7 @@ of order unity in order to decrease the chance of occasionally generating large
 numbers through multiplications which can cause an error above a reasonable
 tolerance.
 
-#### Testing Failure Cases
+#### Testing Failure Cases {#testing_failure_cases}
 
 Adding the "attribute" `// [[OutputRegex, Regular expression to
 match]]` before the `SPECTRE_TEST_CASE` macro will force ctest to only

tests/Unit/Evolution/Actions/Test_RunEventsAndDenseTriggers.cpp

@@ -454,13 +454,18 @@ void test(const bool time_runs_forward) {
           ActionTesting::get_databox<component>(make_not_null(&runner), 0);
       db::mutate<Tags::HistoryEvolvedVariables<variables_tag>>(
           make_not_null(&box),
-          [&time_step_id](const gsl::not_null<History*> history) {
+          [&time_step_id](const gsl::not_null<History*> history,
+                          const TimeStepper& time_stepper) {
             *history = History(3);
-            history->insert(TimeStepId(time_step_id.time_runs_forward(), 0,
-                                       time_step_id.step_time(), 1,
-                                       time_step_id.step_time()),
-                            {1, 1.0});
-          });
+            history->insert(time_step_id, {1, 1.0});
+            history->insert(
+                time_stepper.next_time_id(
+                    time_step_id,
+                    (time_step_id.time_runs_forward() ? 1 : -1) *
+                        time_step_id.step_time().slab().duration() / 4),
+                {1, 1.0});
+          },
+          db::get<Tags::TimeStepper<>>(box));
     }
     if (data_needed) {
       TestCase::check_dense(&runner, true, {});

tests/Unit/Evolution/DgSubcell/Actions/Test_TciAndRollback.cpp

@@ -331,7 +331,7 @@ void test_impl(const bool rdmp_fails, const bool tci_fails,
     get(get<Tags::dt<Var1>>(dt_vars)) =
         (i + 20.0) * get<0>(logical_coordinates(dg_mesh));
     time_stepper_history.insert(
-        {true, 1, Time{Slab{1.0, 2.0}, {static_cast<int>(5 - i), 10}}},
+        {false, 1, Time{Slab{1.0, 2.0}, {static_cast<int>(5 - i), 10}}},
         dt_vars);
   }
   Variables<evolved_vars_tags> vars{dg_mesh.number_of_grid_points()};

tests/Unit/Evolution/DgSubcell/Actions/Test_TciAndSwitchToDg.cpp

@@ -285,7 +285,7 @@ void test_impl(
     get(get<Tags::dt<Var1>>(dt_vars)) =
         (i + 20.0) * get<0>(logical_coordinates(subcell_mesh));
     time_stepper_history.insert(
-        {true, 1, Time{Slab{1.0, 2.0}, {static_cast<int>(5 - i), 10}}},
+        {false, 1, Time{Slab{1.0, 2.0}, {static_cast<int>(5 - i), 10}}},
         dt_vars);
   }
   Variables<evolved_vars_tags> vars{subcell_mesh.number_of_grid_points()};

tests/Unit/Framework/TestingFramework.hpp

@@ -310,6 +310,9 @@ struct check_matrix_approx {
  * we must install a signal handler after Catch does, which means inside the
  * SPECTRE_TEST_CASE itself. The ERROR_TEST() macro should be the first line in
  * the SPECTRE_TEST_CASE.
+ *
+ * \warning This macro is deprecated.  See \ref testing_failure_cases
+ * "the DevGuide" for the modern way to do this.
  */
 #define ERROR_TEST()                                      \
   do {                                                    \
@@ -332,8 +335,8 @@ struct check_matrix_approx {
  * In order to test ASSERT's properly the test must also fail for release
  * builds. This is done by adding this macro at the beginning for the test.
  *
- * \example
- * \snippet Test_Time.cpp example_of_error_test
+ * \warning This macro is deprecated.  See \ref testing_failure_cases
+ * "the DevGuide" for the modern way to do this.
  */
 #ifdef SPECTRE_DEBUG
 #define ASSERTION_TEST() \

tests/Unit/Helpers/Time/TimeSteppers/TimeStepperTestUtils.cpp

@@ -11,6 +11,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <deque>
+#include <fstream>
 #include <limits>
 #include <type_traits>
 
@@ -110,9 +111,9 @@ void check_substep_properties(const TimeStepper& stepper) {
   TimeStepId id(true, 3, slab.start() + slab.duration() / 2);
   TimeSteppers::History<double> history{stepper.order()};
   CHECK(stepper.can_change_step_size(id, history));
+  history.insert(id, 0.0);
   id = stepper.next_time_id(id, slab.duration() / 2);
   if (id.substep() != 0) {
-    history.insert(id, 0.0);
     CHECK(not stepper.can_change_step_size(id, history));
   }
 }
@@ -395,8 +396,19 @@ void equal_rate_boundary(const LtsTimeStepper& stepper, const size_t order,
   CHECK(boundary_history.remote_size() < 20);
 }
 
-void check_convergence_order(const TimeStepper& stepper) {
-  const auto do_integral = [&stepper](const int32_t num_steps) {
+void check_convergence_order(const TimeStepper& stepper,
+                             const std::pair<int32_t, int32_t>& step_range,
+                             const bool output) {
+  // Make sure testing code is not left enabled.
+  CHECK(not output);
+
+  std::ofstream output_stream{};
+  if (output) {
+    output_stream.open("convergence.dat");
+    output_stream.precision(18);
+  }
+  const auto do_integral = [&output, &output_stream,
+                            &stepper](const int32_t num_steps) {
     const Slab slab(0., 1.);
     const TimeDelta step_size = slab.duration() / num_steps;
 
@@ -410,12 +422,13 @@ void check_convergence_order(const TimeStepper& stepper) {
     while (time < slab.end()) {
       take_step(&time, &y, &history, stepper, rhs, step_size);
     }
-    return abs(y - exp(1.));
+    const double result = abs(y - exp(1.));
+    if (output) {
+      output_stream << num_steps << "\t" << result << std::endl;
+    }
+    return result;
   };
-  const int32_t large_steps = 10;
-  // The high-order solvers have round-off error around here
-  const int32_t small_steps = 30;
-  CHECK(convergence_rate(large_steps, small_steps, do_integral) ==
+  CHECK(convergence_rate(step_range.first, step_range.second, do_integral) ==
         approx(stepper.order()).margin(0.4));
 }
 
@@ -436,7 +449,7 @@ void check_dense_output(const TimeStepper& stepper,
           time_id.substep_time(), make_not_null(&history),
           [](const double t) { return exp(t); },
           [](const double v, const double /*t*/) { return v; }, step_size,
-          history_integration_order);
+          stepper.number_of_past_steps());
       auto step = step_size;
       for (;;) {
         history.insert(time_id, y);
@@ -477,7 +490,7 @@ void check_dense_output(const TimeStepper& stepper,
       const auto rhs = [](const double v, const double /*t*/) { return v; };
       initialize_history(
           time, make_not_null(&history), [](const double t) { return exp(t); },
-          rhs, time_step, history_integration_order);
+          rhs, time_step, stepper.number_of_past_steps());
       take_step(&time, &y, &history, stepper, rhs, time_step);
 
       // Some time steppers special-case the endpoints of the

tests/Unit/Helpers/Time/TimeSteppers/TimeStepperTestUtils.hpp

@@ -4,6 +4,8 @@
 #pragma once
 
 #include <cstddef>
+#include <cstdint>
+#include <utility>
 
 #include "Time/History.hpp"
 #include "Time/Slab.hpp"
@@ -70,7 +72,16 @@ void equal_rate_boundary(const LtsTimeStepper& stepper, size_t order,
                          size_t number_of_past_steps, double epsilon,
                          bool forward);
 
-void check_convergence_order(const TimeStepper& stepper);
+/// Check that integration converges as expected.
+///
+/// The \p step_range argument specifies the range of the number of
+/// steps used to produce a fit, and should be cover a factor of a few
+/// over which a log-log plot of the error is roughly linear.  An
+/// appropriate value can be determined by passing `true` as the \p
+/// output argument, which will produce a `convergence.dat` file.
+void check_convergence_order(const TimeStepper& stepper,
+                             const std::pair<int32_t, int32_t>& step_range,
+                             bool output = false);
 
 void check_dense_output(const TimeStepper& stepper,
                         const size_t history_integration_order);

tests/Unit/Time/Actions/Test_RecordTimeStepperData.cpp

@@ -105,73 +105,46 @@ struct Metavariables {
                  ComponentWithTemplateSpecifiedVariables<Metavariables>>;
 };
 
-template <bool UseRollback>
+template <bool UseRollback, template <typename> typename LocalComponent,
+          typename VariablesTag, typename HistoryTag>
 void run_test() {
   const Slab slab(1., 3.);
 
-  history_tag::type history{};
+  typename HistoryTag::type history{};
   history.insert(TimeStepId(true, 0, slab.end()), 3.);
 
-  alternative_history_tag::type alternative_history{};
-  alternative_history.insert(TimeStepId(true, 0, slab.end()), 3.);
-
   using metavariables = Metavariables<UseRollback>;
-  using component = Component<metavariables>;
-  using component_with_template_specified_variables =
-      ComponentWithTemplateSpecifiedVariables<metavariables>;
+  using component = LocalComponent<metavariables>;
   using MockRuntimeSystem = ActionTesting::MockRuntimeSystem<metavariables>;
   MockRuntimeSystem runner{{}};
 
   const double initial_value = 4.;
   ActionTesting::emplace_component_and_initialize<component>(
       &runner, 0,
       {TimeStepId(true, 0, slab.start()), initial_value, 5., std::move(history),
-       Tags::RollbackValue<variables_tag>::type{}});
-  ActionTesting::emplace_component_and_initialize<
-      component_with_template_specified_variables>(
-      &runner, 0,
-      {TimeStepId(true, 0, slab.start()), initial_value, 5.,
-       std::move(alternative_history),
-       Tags::RollbackValue<alternative_variables_tag>::type{}});
+       typename Tags::RollbackValue<VariablesTag>::type{}});
   ActionTesting::set_phase(make_not_null(&runner), Parallel::Phase::Testing);
   runner.template next_action<component>(0);
-  runner.template next_action<component_with_template_specified_variables>(0);
   auto& box = ActionTesting::get_databox<component>(runner, 0);
-  auto& template_specified_variables_box =
-      ActionTesting::get_databox<component_with_template_specified_variables>(
-          runner, 0);
 
-  const auto& new_history = db::get<history_tag>(box);
+  const auto& new_history = db::get<HistoryTag>(box);
   CHECK(new_history.size() == 2);
   CHECK(*new_history.begin() == slab.end());
   CHECK(*new_history.begin().derivative() == 3.);
   CHECK(*(new_history.begin() + 1) == slab.start());
   CHECK(*(new_history.begin() + 1).derivative() == 5.);
   if constexpr (UseRollback) {
-    CHECK(db::get<Tags::RollbackValue<variables_tag>>(box) == initial_value);
-  }
-
-  const auto& new_history_from_template_specified_variables_box =
-      db::get<alternative_history_tag>(template_specified_variables_box);
-  CHECK(new_history_from_template_specified_variables_box.size() == 2);
-  CHECK(*new_history_from_template_specified_variables_box.begin() ==
-        slab.end());
-  CHECK(
-      *new_history_from_template_specified_variables_box.begin().derivative() ==
-      3.);
-  CHECK(*(new_history_from_template_specified_variables_box.begin() + 1) ==
-        slab.start());
-  CHECK(*(new_history_from_template_specified_variables_box.begin() + 1)
-             .derivative() == 5.);
-  if constexpr (UseRollback) {
-    CHECK(db::get<Tags::RollbackValue<alternative_variables_tag>>(
-              template_specified_variables_box) == initial_value);
+    CHECK(db::get<Tags::RollbackValue<VariablesTag>>(box) == initial_value);
   }
 }
 }  // namespace
 
 SPECTRE_TEST_CASE("Unit.Time.Actions.RecordTimeStepperData",
                   "[Unit][Time][Actions]") {
-  run_test<false>();
-  run_test<true>();
+  run_test<false, Component, variables_tag, history_tag>();
+  run_test<false, ComponentWithTemplateSpecifiedVariables,
+           alternative_variables_tag, alternative_history_tag>();
+  run_test<true, Component, variables_tag, history_tag>();
+  run_test<true, ComponentWithTemplateSpecifiedVariables,
+           alternative_variables_tag, alternative_history_tag>();
 }

tests/Unit/Time/Test_TakeStep.cpp

@@ -97,12 +97,12 @@ void test_gts() {
       Tags::Next<Tags::TimeStepId>, Tags::TimeStep, Tags::Next<Tags::TimeStep>,
       EvolvedVariable, Tags::dt<EvolvedVariable>,
       Tags::HistoryEvolvedVariables<EvolvedVariable>,
-      Tags::TimeStepper<LtsTimeStepper>,
+      Tags::TimeStepper<TimeStepper>,
       ::Tags::IsUsingTimeSteppingErrorControl>>(
       Metavariables{}, TimeStepId{true, 0_st, slab.start()},
       TimeStepId{true, 0_st, Time{slab, {1, 4}}}, time_step, time_step,
       initial_values, DataVector{5, 0.0}, std::move(history),
-      static_cast<std::unique_ptr<LtsTimeStepper>>(
+      static_cast<std::unique_ptr<TimeStepper>>(
           std::make_unique<TimeSteppers::AdamsBashforth>(5)),
       false);
   // update the rhs