diff --git a/.clang-tidy b/.clang-tidy index e467891b616d..447cf868cead 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -96,6 +96,9 @@ CheckOptions: value: true - key: performance-move-const-arg.CheckTriviallyCopyableMove value: false + # The fix for this is not supported by GCC 9. + - key: modernize-loop-convert.UseCxx20ReverseRanges + value: false WarningsAsErrors: '*' # It is unclear if the header filter actually works or how to use it so # just include all headers diff --git a/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhBinaryBlackHole.hpp b/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhBinaryBlackHole.hpp index e23a1ff8ac46..655677896a1b 100644 --- a/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhBinaryBlackHole.hpp +++ b/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhBinaryBlackHole.hpp @@ -488,7 +488,9 @@ struct EvolutionMetavars { tmpl::pair< gh::gauges::GaugeCondition, tmpl::list>, - tmpl::pair, + // Restrict to monotonic time steppers in LTS to avoid control + // systems deadlocking. + tmpl::pair, tmpl::pair, tmpl::pair, StepChoosers::standard_step_choosers>, diff --git a/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhSingleBlackHole.hpp b/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhSingleBlackHole.hpp index 41fecb87508c..15f7daa9e3a6 100644 --- a/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhSingleBlackHole.hpp +++ b/src/Evolution/Executables/GeneralizedHarmonic/EvolveGhSingleBlackHole.hpp @@ -177,7 +177,13 @@ struct EvolutionMetavars : public GeneralizedHarmonicTemplateBase<3, UseLts> { struct factory_creation : tt::ConformsTo { using factory_classes = Options::add_factory_classes< - typename gh_base::factory_creation::factory_classes, + // Restrict to monotonic time steppers in LTS to avoid control + // systems deadlocking. + tmpl::insert< + tmpl::erase, + tmpl::pair>, tmpl::pair, tmpl::pair, tmpl::pair, - tmpl::pair, + // Restrict to monotonic time steppers in LTS to avoid control + // systems deadlocking. + tmpl::pair< + LtsTimeStepper, + tmpl::conditional_t>, tmpl::pair, tmpl::pair, StepChoosers::standard_step_choosers>, diff --git a/src/Evolution/Executables/ScalarTensor/EvolveScalarTensorSingleBlackHole.hpp b/src/Evolution/Executables/ScalarTensor/EvolveScalarTensorSingleBlackHole.hpp index 8cef0b370e03..af38b458ff5f 100644 --- a/src/Evolution/Executables/ScalarTensor/EvolveScalarTensorSingleBlackHole.hpp +++ b/src/Evolution/Executables/ScalarTensor/EvolveScalarTensorSingleBlackHole.hpp @@ -154,7 +154,13 @@ struct EvolutionMetavars : public ScalarTensorTemplateBase { struct factory_creation : tt::ConformsTo { using factory_classes = Options::add_factory_classes< - typename st_base::factory_creation::factory_classes, + // Restrict to monotonic time steppers in LTS to avoid control + // systems deadlocking. + tmpl::insert< + tmpl::erase, + tmpl::pair>, tmpl::pair*> box) { << time_step_id.substep_time() << "."); } - const auto& next_time_id = db::get>(*box); - const auto new_step = - choose_lts_step_size(next_time_id.step_time(), desired_step); + const auto new_step = choose_lts_step_size( + time_step_id.step_time() + current_step, desired_step); db::mutate>( [&new_step](const gsl::not_null next_step) { *next_step = new_step; diff --git a/src/Time/BoundaryHistory.hpp b/src/Time/BoundaryHistory.hpp index 11940e4ad64f..9963470ec657 100644 --- a/src/Time/BoundaryHistory.hpp +++ b/src/Time/BoundaryHistory.hpp @@ -15,7 +15,6 @@ #include "DataStructures/CircularDeque.hpp" #include "DataStructures/MathWrapper.hpp" -#include "DataStructures/StaticDeque.hpp" #include "Time/History.hpp" #include "Time/TimeStepId.hpp" #include "Utilities/Algorithm.hpp" @@ -342,20 +341,16 @@ class BoundaryHistory { void clear_substeps_local(size_t n); void clear_substeps_remote(size_t n); - StaticDeque, history_max_past_steps + 2> local_data_{}; + CircularDeque> local_data_{}; CircularDeque> remote_data_{}; template using CouplingSubsteps = boost::container::static_vector; - // Putting the CircularDeque outermost means that we are inserting - // and removing containers that do not allocate, so we don't have to - // worry about that. // NOLINTNEXTLINE(spectre-mutable) mutable CircularDeque>, - decltype(local_data_)::max_size()>>> + CircularDeque>>>> couplings_; }; diff --git a/src/Time/TimeSteppers/AdamsBashforth.cpp b/src/Time/TimeSteppers/AdamsBashforth.cpp index ea59e0b401b5..e7c05f0225a4 100644 --- a/src/Time/TimeSteppers/AdamsBashforth.cpp +++ b/src/Time/TimeSteppers/AdamsBashforth.cpp @@ -4,15 +4,12 @@ #include "Time/TimeSteppers/AdamsBashforth.hpp" #include -#include #include #include +#include #include -#include #include -#include -#include "NumericalAlgorithms/Interpolation/LagrangePolynomial.hpp" #include "Time/ApproximateTime.hpp" #include "Time/BoundaryHistory.hpp" #include "Time/EvolutionOrdering.hpp" @@ -21,6 +18,7 @@ #include "Time/Time.hpp" #include "Time/TimeStepId.hpp" #include "Time/TimeSteppers/AdamsCoefficients.hpp" +#include "Time/TimeSteppers/AdamsLts.hpp" #include "Utilities/Algorithm.hpp" #include "Utilities/ErrorHandling/Assert.hpp" #include "Utilities/ErrorHandling/Error.hpp" @@ -223,8 +221,16 @@ void AdamsBashforth::add_boundary_delta_impl( const TimeSteppers::ConstBoundaryHistoryTimes& remote_times, const TimeSteppers::BoundaryHistoryEvaluator& coupling, const TimeDelta& time_step) const { - boundary_impl(result, local_times, remote_times, coupling, - local_times.back().step_time() + time_step); + const auto step_start = local_times.back().step_time(); + const size_t integration_order = + local_times.integration_order(local_times.size() - 1); + + const adams_lts::AdamsScheme scheme{adams_lts::SchemeType::Explicit, + integration_order}; + const auto lts_coefficients = adams_lts::lts_coefficients( + local_times, remote_times, step_start, step_start + time_step, scheme, + scheme, scheme); + adams_lts::apply_coefficients(result, lts_coefficients, coupling); } void AdamsBashforth::clean_boundary_history_impl( @@ -256,362 +262,20 @@ void AdamsBashforth::boundary_dense_output_impl( // which is the input value of `result`. return; } - return boundary_impl(result, local_times, remote_times, coupling, - ApproximateTime{time}); -} - -namespace { -template -class SmallStepIterator { - public: - using iterator_category = std::forward_iterator_tag; - using value_type = Time; - using pointer = const Time*; - using reference = const Time&; - using difference_type = std::ptrdiff_t; - - enum class Side { Local, Remote, Both }; - - using history_iterator = typename ConstBoundaryHistoryTimes::const_iterator; - - SmallStepIterator() = default; - - SmallStepIterator(history_iterator local_begin, history_iterator remote_begin, - history_iterator local_end, history_iterator remote_end) - : local_id_(std::move(local_begin)), - remote_id_(std::move(remote_begin)), - local_end_(std::move(local_end)), - remote_end_(std::move(remote_end)) {} - - reference operator*() const { - return std::max(*local_id_, *remote_id_).step_time(); - } - pointer operator->() const { return &**this; } - - Side side() const { - if (*local_id_ < *remote_id_) { - return Side::Remote; - } else if (*remote_id_ < *local_id_) { - return Side::Local; - } else { - return Side::Both; - } - } - - // These are the m^s(n) in the paper. - const history_iterator& local_iterator() const { return local_id_; } - const history_iterator& remote_iterator() const { return remote_id_; } - - SmallStepIterator& operator++() { - ASSERT(local_id_ != local_end_ and remote_id_ != remote_end_, - "Overran iterator"); - auto local_candidate = std::next(local_id_); - auto remote_candidate = std::next(remote_id_); - - // NOLINTNEXTLINE(bugprone-branch-clone) - if (local_candidate == local_end_ and remote_candidate == remote_end_) { - local_id_ = std::move(local_candidate); - remote_id_ = std::move(remote_candidate); - // NOLINTNEXTLINE(bugprone-branch-clone) - } else if (local_candidate == local_end_) { - remote_id_ = std::move(remote_candidate); - // NOLINTNEXTLINE(bugprone-branch-clone) - } else if (remote_candidate == remote_end_) { - local_id_ = std::move(local_candidate); - } else if (*local_candidate < *remote_candidate) { - local_id_ = std::move(local_candidate); - } else if (*remote_candidate < *local_candidate) { - remote_id_ = std::move(remote_candidate); - } else { - local_id_ = std::move(local_candidate); - remote_id_ = std::move(remote_candidate); - } - - return *this; - } - - bool done() const { - return local_id_ == local_end_ and remote_id_ == remote_end_; - } - - private: - history_iterator local_id_{}; - history_iterator remote_id_{}; - history_iterator local_end_{}; - history_iterator remote_end_{}; -}; - -template -bool operator==(const SmallStepIterator& a, const SmallStepIterator& b) { - if (a.done() and b.done()) { - return true; - } - if (a.done() or b.done()) { - return false; - } - return *a == *b; -} - -template -bool operator!=(const SmallStepIterator& a, const SmallStepIterator& b) { - return not(a == b); -} - -template -bool operator<(const SmallStepIterator& a, const SmallStepIterator& b) { - return a.local_iterator() < b.local_iterator() or - a.remote_iterator() < b.remote_iterator(); -} - -template -bool operator>(const SmallStepIterator& a, const SmallStepIterator& b) { - return b < a; -} - -template -It bounded_next_impl(std::input_iterator_tag /*meta*/, It it, const It& bound, - typename std::iterator_traits::difference_type n) { - ASSERT(n >= 0, "Can't advance an input iterator backwards."); - for (typename std::iterator_traits::difference_type i = 0; - i < n and it != bound; ++i, ++it) { - } - return it; -} - -template -It bounded_next_impl(std::random_access_iterator_tag /*meta*/, const It& it, - typename std::iterator_traits::difference_type n, - const It& bound) { - if (bound - it < n) { - return bound; - } else { - return it + n; - } -} - -template -It bounded_next(const It& it, const It& bound, const size_t n) { - return bounded_next_impl( - typename std::iterator_traits::iterator_category{}, it, bound, - static_cast::difference_type>(n)); -} -} // namespace - -template -void AdamsBashforth::boundary_impl( - const gsl::not_null result, - const ConstBoundaryHistoryTimes& local_times, - const ConstBoundaryHistoryTimes& remote_times, - const BoundaryHistoryEvaluator& coupling, - const TimeType& end_time) const { - // Might be different from order_ during self-start. const auto current_order = local_times.integration_order(local_times.size() - 1); - - ASSERT(current_order <= order_, - "Local history is too long for target order (" << current_order - << " should not exceed " << order_ << ")"); - ASSERT(remote_times.size() >= current_order, - "Remote history is too short (" << remote_times.size() - << " should be at least " << current_order << ")"); - - // Avoid billions of casts - const auto order_s = static_cast< - typename ConstBoundaryHistoryTimes::iterator::difference_type>( - current_order); - - // Start and end of the step we are trying to take - const Time start_time = local_times.back().step_time(); - const auto time_step = end_time - start_time; - - ASSERT(local_times.size() == current_order, - "Incorrect local data, have " << local_times.size() << " entry, need " - << current_order); - - if (std::equal(local_times.begin(), local_times.end(), - remote_times.end() - order_s)) { - // No local time-stepping going on. - OrderVector