From 68928c326a8486d72aa452b2fc272b61a4f5cf8e Mon Sep 17 00:00:00 2001 From: Kyle Nelli Date: Thu, 5 Oct 2023 13:15:32 -0700 Subject: [PATCH] fixup --- .../Callbacks/FindApparentHorizon.hpp | 5 +-- .../Actions/InterpolatorReceivePoints.hpp | 9 ++++++ .../Test_InterpolatorReceivePoints.cpp | 32 +++++++++++++++---- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/ParallelAlgorithms/ApparentHorizonFinder/Callbacks/FindApparentHorizon.hpp b/src/ParallelAlgorithms/ApparentHorizonFinder/Callbacks/FindApparentHorizon.hpp index e656bc46396e3..9064027eeae22 100644 --- a/src/ParallelAlgorithms/ApparentHorizonFinder/Callbacks/FindApparentHorizon.hpp +++ b/src/ParallelAlgorithms/ApparentHorizonFinder/Callbacks/FindApparentHorizon.hpp @@ -267,10 +267,11 @@ struct FindApparentHorizon auto& interpolation_target = Parallel::get_parallel_component< intrp::InterpolationTarget>( *cache); - // The iteration of these new coords is the fast flow iteration + // The iteration of these new coords is the fast flow iteration + 1 + // because the zeroth iteration was the initial guess Parallel::simple_action< Actions::SendPointsToInterpolator>( - interpolation_target, temporal_ids.front(), info.iteration); + interpolation_target, temporal_ids.front(), info.iteration + 1); // We return false because we don't want this iteration to clean // up the volume data, since we are using it for the next iteration // (i.e. the simple_action that we just called). diff --git a/src/ParallelAlgorithms/Interpolation/Actions/InterpolatorReceivePoints.hpp b/src/ParallelAlgorithms/Interpolation/Actions/InterpolatorReceivePoints.hpp index 46ee3fa9234a5..9347c1764284c 100644 --- a/src/ParallelAlgorithms/Interpolation/Actions/InterpolatorReceivePoints.hpp +++ b/src/ParallelAlgorithms/Interpolation/Actions/InterpolatorReceivePoints.hpp @@ -12,6 +12,7 @@ #include "DataStructures/Tensor/TypeAliases.hpp" #include "ParallelAlgorithms/Interpolation/Actions/TryToInterpolate.hpp" #include "ParallelAlgorithms/Interpolation/InterpolatedVars.hpp" +#include "Utilities/ErrorHandling/Error.hpp" #include "Utilities/Gsl.hpp" #include "Utilities/Requires.hpp" #include "Utilities/TaggedTuple.hpp" @@ -53,6 +54,9 @@ namespace Actions { /// recent" or "older" so if we receive an older set of points after a more /// recent set, we don't overwrite the more recent set. /// +/// \note If the interpolator receives points with the same iteration, an ERROR +/// will occur. +/// /// Uses: /// - Databox: /// - `Tags::NumberOfElements` @@ -116,6 +120,11 @@ struct ReceivePoints { intrp::Vars::Info{ std::move(block_logical_coords), iteration}); + } else if (vars_infos.at(temporal_id).iteration == iteration) { + ERROR( + "Interpolator received target points at iteration " + << iteration + << " twice. Only one set of points per iteration is allowed."); } }, make_not_null(&box)); diff --git a/tests/Unit/ParallelAlgorithms/Interpolation/Test_InterpolatorReceivePoints.cpp b/tests/Unit/ParallelAlgorithms/Interpolation/Test_InterpolatorReceivePoints.cpp index d14356b59b0b7..f1df6e39d0b6f 100644 --- a/tests/Unit/ParallelAlgorithms/Interpolation/Test_InterpolatorReceivePoints.cpp +++ b/tests/Unit/ParallelAlgorithms/Interpolation/Test_InterpolatorReceivePoints.cpp @@ -313,11 +313,13 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", holders); }; - // Send points to both interpolator cores. Not much should happen + // Send points to both interpolator cores. Not much should happen. We send at + // iteration 1 so we can test receiving points before, at, and after this + // iteration. for (size_t array_index = 0; array_index < 2; array_index++) { runner.simple_action>( - array_index, temporal_id, block_logical_coords, 0_st); + array_index, temporal_id, block_logical_coords, 1_st); const auto& holder = get_holder(array_index); // Should now be a single info in holder, indexed by temporal_id. @@ -329,8 +331,9 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", CHECK(info.interpolation_is_done_for_these_elements.empty()); CHECK(info.global_offsets.empty()); CHECK(info.vars.empty()); - // But block_coord_holders should be filled. + // But block_coord_holders should be filled and the iteration should be 1 CHECK(info.block_coord_holders == block_logical_coords); + CHECK(info.iteration == 1_st); // There should be no more queued actions; verify this. CHECK(runner.is_simple_action_queue_empty(array_index)); // Make sure that the action was not called. @@ -366,6 +369,7 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", CHECK(info.interpolation_is_done_for_these_elements.count(element_ids[i]) == 1); CHECK(info.block_coord_holders == block_logical_coords); + CHECK(info.iteration == 1_st); CHECK(info.global_offsets.size() == i + 1); CHECK(info.vars.size() == i + 1); @@ -424,6 +428,7 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", CHECK(info.interpolation_is_done_for_these_elements.count(element_ids[i]) == 1); CHECK(info.block_coord_holders == block_logical_coords); + CHECK(info.iteration == 1_st); CHECK(info.global_offsets.empty()); CHECK(info.vars.empty()); @@ -449,7 +454,7 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", } // Send data to second interpolator core from the seventh element. We should - // not have done an interpolation to these new points because of iteration 0 + // not have done an interpolation to these new points because of iteration 1 runner.simple_action< interp_component, intrp::Actions::InterpolatorReceiveVolumeData>( @@ -472,6 +477,7 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", 1); // These should still be the old coords CHECK_FALSE(info.block_coord_holders == block_logical_coords); + CHECK(info.iteration == 1_st); CHECK(info.global_offsets.empty()); CHECK(info.vars.empty()); @@ -481,12 +487,25 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", CHECK(num_calls_of_target_receive_vars == 1); } - // Now send with iteration 1. These points should overwrite the previous ones + // Now send with iteration 1. One core 0, this shouldn't error because we have + // cleaned up points. But on core 1 this should error + runner.simple_action>( + 0_st, temporal_id, block_logical_coords, 1_st); + CHECK_THROWS_WITH( + (runner.simple_action< + interp_component, + intrp::Actions::ReceivePoints>( + 1_st, temporal_id, block_logical_coords, 1_st)), + Catch::Matchers::ContainsSubstring( + "Interpolator received target points at iteration 1 twice.")); + + // Now send with iteration 2. These points should overwrite the previous ones // and an interpolation should happen for (size_t array_index = 0; array_index < 2; array_index++) { runner.simple_action>( - array_index, temporal_id, block_logical_coords, 1_st); + array_index, temporal_id, block_logical_coords, 2_st); } { @@ -505,6 +524,7 @@ SPECTRE_TEST_CASE("Unit.NumericalAlgorithms.Interpolator.ReceivePoints", CHECK(info.interpolation_is_done_for_these_elements.count(element_ids[6]) == 1); CHECK(info.block_coord_holders == block_logical_coords); + CHECK(info.iteration == 2_st); CHECK(info.global_offsets.size() == 3); CHECK(info.vars.size() == 3);