From ce959e2c2927cb792e25981717062020555c2317 Mon Sep 17 00:00:00 2001 From: William Throwe Date: Fri, 13 Dec 2024 15:12:48 -0500 Subject: [PATCH 1/2] Make FoT algorithm_callback handle nodegroups Instead of every caller embedding the nodegroup check logic, move it inside the function. Removed the call from the Interpolate event as FoTs are guaranteed to be ready there. --- .../Actions/FunctionsOfTimeAreReady.hpp | 110 ++++++++---------- .../Interpolation/Events/Interpolate.hpp | 28 +---- .../Test_FunctionsOfTimeAreReady.cpp | 29 +++-- .../Interpolation/Test_Interpolate.cpp | 7 -- 4 files changed, 69 insertions(+), 105 deletions(-) diff --git a/src/ParallelAlgorithms/Actions/FunctionsOfTimeAreReady.hpp b/src/ParallelAlgorithms/Actions/FunctionsOfTimeAreReady.hpp index 2a7d444509d6..6ba4f34040fe 100644 --- a/src/ParallelAlgorithms/Actions/FunctionsOfTimeAreReady.hpp +++ b/src/ParallelAlgorithms/Actions/FunctionsOfTimeAreReady.hpp @@ -119,29 +119,6 @@ bool functions_of_time_are_ready_impl( } } // namespace detail -/// \ingroup ComputationalDomainGroup -/// Check that functions of time are up-to-date. -/// -/// Check that functions of time in \p CacheTag with names in \p -/// functions_to_check are ready at time \p time. If \p functions_to_check is -/// a `std::nullopt`, checks all functions in \p CacheTag. If any function is -/// not ready, schedules a `Parallel::PerformAlgorithmCallback` with the -/// GlobalCache.. -template -bool functions_of_time_are_ready_algorithm_callback( - Parallel::GlobalCache& cache, const ArrayIndex& array_index, - const Component* component_p, const double time, - const std::optional>& functions_to_check = - std::nullopt) { - using ProxyType = - std::decay_t( - cache)[array_index])>; - return detail::functions_of_time_are_ready_impl< - CacheTag, Parallel::PerformAlgorithmCallback>( - cache, array_index, component_p, time, functions_to_check); -} - /// \ingroup ComputationalDomainGroup /// Check that functions of time are up-to-date. /// @@ -194,6 +171,47 @@ bool functions_of_time_are_ready_threaded_action_callback( std::forward(args)...); } +/// \ingroup ComputationalDomainGroup +/// Check that functions of time are up-to-date. +/// +/// Check that functions of time in \p CacheTag with names in \p +/// functions_to_check are ready at time \p time. If \p functions_to_check is +/// a `std::nullopt`, checks all functions in \p CacheTag. If any function is +/// not ready, schedules a `Parallel::PerformAlgorithmCallback` or +/// `Parallel::Actions::PerformAlgorithmOnElement` callback with the +/// GlobalCache. +template +bool functions_of_time_are_ready_algorithm_callback( + Parallel::GlobalCache& cache, const ArrayIndex& array_index, + const Component* component_p, const double time, + const std::optional>& functions_to_check = + std::nullopt) { + if constexpr (Parallel::is_dg_element_collection_v) { + const auto element_location = + static_cast(Parallel::local_synchronous_action< + Parallel::Actions::GetItemFromDistributedOject< + Parallel::Tags::ElementLocations>>( + Parallel::get_parallel_component(cache)) + ->at(array_index)); + ASSERT(element_location == Parallel::my_node(cache), + "Expected to be running on node " + << Parallel::my_node(cache) + << " but the record says it is on node " << element_location); + return functions_of_time_are_ready_threaded_action_callback< + domain::Tags::FunctionsOfTime, + Parallel::Actions::PerformAlgorithmOnElement>( + cache, element_location, component_p, time, std::nullopt, array_index); + } else { + using ProxyType = + std::decay_t( + cache)[array_index])>; + return detail::functions_of_time_are_ready_impl< + CacheTag, Parallel::PerformAlgorithmCallback>( + cache, array_index, component_p, time, functions_to_check); + } +} + namespace Actions { /// \ingroup ComputationalDomainGroup /// Check that functions of time are up-to-date. @@ -212,29 +230,9 @@ struct CheckFunctionsOfTimeAreReady { Parallel::GlobalCache& cache, const ArrayIndex& array_index, ActionList /*meta*/, const ParallelComponent* component) { - bool ready = false; - - if constexpr (Parallel::is_dg_element_collection_v) { - const auto element_location = static_cast( - Parallel::local_synchronous_action< - Parallel::Actions::GetItemFromDistributedOject< - Parallel::Tags::ElementLocations>>( - Parallel::get_parallel_component(cache)) - ->at(array_index)); - ASSERT(element_location == Parallel::my_node(cache), - "Expected to be running on node " - << Parallel::my_node(cache) - << " but the record says it is on node " << element_location); - ready = domain::functions_of_time_are_ready_threaded_action_callback< - domain::Tags::FunctionsOfTime, - Parallel::Actions::PerformAlgorithmOnElement>( - cache, element_location, component, db::get<::Tags::Time>(box), - std::nullopt, array_index); - } else { - ready = functions_of_time_are_ready_algorithm_callback< - domain::Tags::FunctionsOfTime>(cache, array_index, component, - db::get<::Tags::Time>(box)); - } + const bool ready = functions_of_time_are_ready_algorithm_callback< + domain::Tags::FunctionsOfTime, Dim>(cache, array_index, component, + db::get<::Tags::Time>(box)); return {ready ? Parallel::AlgorithmExecution::Continue : Parallel::AlgorithmExecution::Retry, @@ -264,23 +262,9 @@ struct CheckFunctionsOfTimeAreReadyPostprocessor { const gsl::not_null*> /*inboxes*/, Parallel::GlobalCache& cache, const ArrayIndex& array_index, const ParallelComponent* component) { - if constexpr (Parallel::is_dg_element_collection_v) { - const auto element_location = static_cast( - Parallel::local_synchronous_action< - Parallel::Actions::GetItemFromDistributedOject< - Parallel::Tags::ElementLocations>>( - Parallel::get_parallel_component(cache)) - ->at(array_index)); - return domain::functions_of_time_are_ready_threaded_action_callback< - domain::Tags::FunctionsOfTime, - Parallel::Actions::PerformAlgorithmOnElement>( - cache, element_location, component, db::get<::Tags::Time>(*box), - std::nullopt, array_index); - } else { - return functions_of_time_are_ready_algorithm_callback< - domain::Tags::FunctionsOfTime>(cache, array_index, component, - db::get<::Tags::Time>(*box)); - } + return functions_of_time_are_ready_algorithm_callback< + domain::Tags::FunctionsOfTime, Dim>(cache, array_index, component, + db::get<::Tags::Time>(*box)); } }; } // namespace domain diff --git a/src/ParallelAlgorithms/Interpolation/Events/Interpolate.hpp b/src/ParallelAlgorithms/Interpolation/Events/Interpolate.hpp index 482277c796c6..a304ae50f1d0 100644 --- a/src/ParallelAlgorithms/Interpolation/Events/Interpolate.hpp +++ b/src/ParallelAlgorithms/Interpolation/Events/Interpolate.hpp @@ -13,7 +13,6 @@ #include "Parallel/ArrayCollection/PerformAlgorithmOnElement.hpp" #include "Parallel/ArrayCollection/Tags/ElementLocations.hpp" #include "Parallel/GlobalCache.hpp" -#include "ParallelAlgorithms/Actions/FunctionsOfTimeAreReady.hpp" #include "ParallelAlgorithms/Actions/GetItemFromDistributedObject.hpp" #include "ParallelAlgorithms/EventsAndTriggers/Event.hpp" #include "ParallelAlgorithms/Interpolation/Events/GetComputeItemsOnSource.hpp" @@ -34,9 +33,6 @@ class GlobalCache; namespace Tags { struct Time; } // namespace Tags -namespace domain::Tags { -struct FunctionsOfTime; -} // namespace domain::Tags namespace Events::Tags { template struct ObserverMesh; @@ -111,27 +107,13 @@ class Interpolate; + using is_ready_argument_tags = tmpl::list<>; template - bool is_ready(const double time, Parallel::GlobalCache& cache, - const ArrayIndex& array_index, - const Component* const component) const { - if constexpr (Parallel::is_dg_element_collection_v) { - const auto element_location = static_cast( - Parallel::local_synchronous_action< - Parallel::Actions::GetItemFromDistributedOject< - Parallel::Tags::ElementLocations>>( - Parallel::get_parallel_component(cache)) - ->at(array_index)); - return domain::functions_of_time_are_ready_threaded_action_callback< - domain::Tags::FunctionsOfTime, - Parallel::Actions::PerformAlgorithmOnElement>( - cache, element_location, component, time, std::nullopt, array_index); - } else { - return domain::functions_of_time_are_ready_algorithm_callback< - domain::Tags::FunctionsOfTime>(cache, array_index, component, time); - } + bool is_ready(Parallel::GlobalCache& /*cache*/, + const ArrayIndex& /*array_index*/, + const Component* const /*component*/) const { + return true; } bool needs_evolved_variables() const override { return true; } diff --git a/tests/Unit/Domain/FunctionsOfTime/Test_FunctionsOfTimeAreReady.cpp b/tests/Unit/Domain/FunctionsOfTime/Test_FunctionsOfTimeAreReady.cpp index 70799a30f117..b1ca24ce339a 100644 --- a/tests/Unit/Domain/FunctionsOfTime/Test_FunctionsOfTimeAreReady.cpp +++ b/tests/Unit/Domain/FunctionsOfTime/Test_FunctionsOfTimeAreReady.cpp @@ -4,6 +4,7 @@ #include "Framework/TestingFramework.hpp" #include +#include #include #include #include @@ -138,6 +139,8 @@ SPECTRE_TEST_CASE("Unit.Domain.FunctionsOfTimeAreReady", "[Domain][Unit]") { using component1 = Component; const component0* const component_0_p = nullptr; const component1* const component_1_p = nullptr; + // Not used when not using nodegroup element components + constexpr size_t dim = std::numeric_limits::max(); const std::array fot_init{{{0.0}, {0.0}, {0.0}}}; FunctionMap functions_of_time{}; @@ -175,29 +178,31 @@ SPECTRE_TEST_CASE("Unit.Domain.FunctionsOfTimeAreReady", "[Domain][Unit]") { INFO("Testing perform algorithm callback support"); // Neither function ready CHECK(not domain::functions_of_time_are_ready_algorithm_callback< - OtherFunctionsOfTime>(cache, 0, component_0_p, 0.5, std::nullopt)); + OtherFunctionsOfTime, dim>(cache, 0, component_0_p, 0.5, + std::nullopt)); CHECK(not domain::functions_of_time_are_ready_algorithm_callback< - OtherFunctionsOfTime>(cache, 0, component_0_p, 0.5, - std::unordered_set{"OtherA"s})); + OtherFunctionsOfTime, dim>(cache, 0, component_0_p, 0.5, + std::unordered_set{"OtherA"s})); // Make OtherA ready Parallel::mutate(cache, "OtherA"s, 123.0); CHECK(domain::functions_of_time_are_ready_algorithm_callback< - OtherFunctionsOfTime>(cache, 0, component_0_p, 0.5, - std::unordered_set{"OtherA"s})); + OtherFunctionsOfTime, dim>(cache, 0, component_0_p, 0.5, + std::unordered_set{"OtherA"s})); CHECK(not domain::functions_of_time_are_ready_algorithm_callback< - OtherFunctionsOfTime>(cache, 0, component_0_p, 0.5, - std::unordered_set{"OtherA"s, "OtherB"s})); + OtherFunctionsOfTime, dim>(cache, 0, component_0_p, 0.5, + std::unordered_set{"OtherA"s, "OtherB"s})); // Make OtherB ready Parallel::mutate(cache, "OtherB"s, 456.0); CHECK(domain::functions_of_time_are_ready_algorithm_callback< - OtherFunctionsOfTime>(cache, 0, component_0_p, 0.5, - std::unordered_set{"OtherA"s, "OtherB"s})); + OtherFunctionsOfTime, dim>(cache, 0, component_0_p, 0.5, + std::unordered_set{"OtherA"s, "OtherB"s})); CHECK(domain::functions_of_time_are_ready_algorithm_callback< - OtherFunctionsOfTime>(cache, 0, component_0_p, 0.5, std::nullopt)); + OtherFunctionsOfTime, dim>(cache, 0, component_0_p, 0.5, + std::nullopt)); } // Test the action. This should automatically look at @@ -480,8 +485,8 @@ SPECTRE_TEST_CASE("Unit.Domain.FunctionsOfTimeAreReady", "[Domain][Unit]") { auto& empty_cache = ActionTesting::cache(empty_runner, 0); CHECK(domain::functions_of_time_are_ready_algorithm_callback< - domain::Tags::FunctionsOfTime>(empty_cache, 0, empty_component_p, - 6.0)); + domain::Tags::FunctionsOfTime, dim>(empty_cache, 0, empty_component_p, + 6.0)); CHECK(domain::functions_of_time_are_ready_simple_action_callback< domain::Tags::FunctionsOfTime, SimpleActionNoArgs>( empty_cache, 0, empty_component_p, 6.0, std::nullopt)); diff --git a/tests/Unit/ParallelAlgorithms/Interpolation/Test_Interpolate.cpp b/tests/Unit/ParallelAlgorithms/Interpolation/Test_Interpolate.cpp index 502af6e9f5ae..45d88c1ca87e 100644 --- a/tests/Unit/ParallelAlgorithms/Interpolation/Test_Interpolate.cpp +++ b/tests/Unit/ParallelAlgorithms/Interpolation/Test_Interpolate.cpp @@ -289,10 +289,6 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.InterpolateEvent", metavars::event event{}; - // Functions of time aren't ready yet. - CHECK_FALSE(static_cast(event).is_ready( - box, cache, array_index, std::add_pointer_t{})); - // Update time in box and functions of time db::mutate<::Tags::Time>([](gsl::not_null time) { *time = 1.0; }, make_not_null(&box)); @@ -300,9 +296,6 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.InterpolateEvent", control_system::UpdateSingleFunctionOfTime>( cache, name, initial_expr_time, DataVector{1, 0.0}, observation_time); - // Now everything should be ready - CHECK(static_cast(event).is_ready( - box, cache, array_index, std::add_pointer_t{})); CHECK(event.needs_evolved_variables()); auto obs_box = make_observation_box< From dacb94c130e1029515cf5ca71ff3fb5715c4cb9f Mon Sep 17 00:00:00 2001 From: William Throwe Date: Fri, 13 Dec 2024 15:18:36 -0500 Subject: [PATCH 2/2] Check FoTs are ready in ObserveConstantsPerElement Not guaranteed for events that do not use the evolved variables. --- .../Events/ObserveConstantsPerElement.hpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/ParallelAlgorithms/Events/ObserveConstantsPerElement.hpp b/src/ParallelAlgorithms/Events/ObserveConstantsPerElement.hpp index dbde24638441..9c903f405991 100644 --- a/src/ParallelAlgorithms/Events/ObserveConstantsPerElement.hpp +++ b/src/ParallelAlgorithms/Events/ObserveConstantsPerElement.hpp @@ -25,6 +25,7 @@ #include "Parallel/Invoke.hpp" #include "Parallel/Local.hpp" #include "Parallel/TypeTraits.hpp" +#include "ParallelAlgorithms/Actions/FunctionsOfTimeAreReady.hpp" #include "ParallelAlgorithms/EventsAndTriggers/Event.hpp" #include "Utilities/TMPL.hpp" @@ -34,6 +35,9 @@ class Domain; namespace domain::FunctionsOfTime { class FunctionOfTime; } // namespace domain::FunctionsOfTime +namespace domain::Tags { +struct FunctionsOfTime; +} // namespace domain::Tags namespace observers { class ObservationKey; enum class TypeOfObservation; @@ -45,6 +49,9 @@ class GlobalCache; namespace PUP { class er; } // namespace PUP +namespace Tags { +struct Time; +} // namespace Tags /// \endcond namespace dg::Events { @@ -98,13 +105,15 @@ class ObserveConstantsPerElement : public Event { std::pair> get_observation_type_and_key_for_registration() const; - using is_ready_argument_tags = tmpl::list<>; + using is_ready_argument_tags = tmpl::list<::Tags::Time>; template - bool is_ready(Parallel::GlobalCache& /*cache*/, - const ArrayIndex& /*array_index*/, - const Component* const /*meta*/) const { - return true; + bool is_ready(const double time, Parallel::GlobalCache& cache, + const ArrayIndex& array_index, + const Component* const component) const { + return ::domain::functions_of_time_are_ready_algorithm_callback< + ::domain::Tags::FunctionsOfTime, VolumeDim>(cache, array_index, + component, time); } void pup(PUP::er& p) override;