Merge branch 'fix-sweep-copy-bug' into 'master'

Fix unintended sweep copying introduced in !486

See merge request walberla/walberla!514

src/blockforest/CMakeLists.txt

@@ -5,7 +5,7 @@ mark_as_advanced( WALBERLA_BLOCKFOREST_PRIMITIVE_BLOCKID )
 
 configure_file( CMakeDefs.in.h CMakeDefs.h )
 
-add_library( blockforest )
+add_library( blockforest ../../tests/timeloop/TimeloopSweepManagementTest.cpp)
 target_link_libraries( blockforest PUBLIC communication core domain_decomposition stencil )
 target_sources( blockforest
       PRIVATE

src/core/selectable/SelectableObject.h

 //======================================================================================================================
 //
-//  This file is part of waLBerla. waLBerla is free software: you can 
+//  This file is part of waLBerla. waLBerla is free software: you can
 //  redistribute it and/or modify it under the terms of the GNU General Public
-//  License as published by the Free Software Foundation, either version 3 of 
+//  License as published by the Free Software Foundation, either version 3 of
 //  the License, or (at your option) any later version.
-//  
-//  waLBerla is distributed in the hope that it will be useful, but WITHOUT 
-//  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 
-//  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
+//
+//  waLBerla is distributed in the hope that it will be useful, but WITHOUT
+//  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+//  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 //  for more details.
-//  
+//
 //  You should have received a copy of the GNU General Public License along
 //  with waLBerla (see COPYING.txt). If not, see <http://www.gnu.org/licenses/>.
 //
@@ -148,6 +148,8 @@ public:
    iterator       end()       { return       iterator( this, object_.size() ); }
    const_iterator end() const { return const_iterator( this, object_.size() ); }
 
+   size_t getNumberOfMatching( const S& selector ) const;
+
    size_t get(              T& object, const S& selector ) const;
    void   get( std::vector<T>& object, const S& selector ) const;
 
@@ -202,7 +204,24 @@ void SelectableObject<T,A,S>::add( const T& object, const A& attributes, const s
    attributes_.push_back( attributes );
 }
 
+//**********************************************************************************************************************
+/*!
+*   Returns the number of objects matching the specified "selector".
+ */
+//**********************************************************************************************************************
+template< typename T, typename A, typename S >
+size_t SelectableObject<T,A,S>::getNumberOfMatching( const S& selector ) const {
+
+   std::vector< size_t > index;
 
+   select( index, selector );
+
+   if( !index.empty() ) {
+      WALBERLA_ASSERT_LESS( index[0], object_.size() );
+   }
+
+   return index.size();
+}
 
 //**********************************************************************************************************************
 /*!

src/timeloop/SweepTimeloop.cpp

@@ -16,6 +16,7 @@
 //! \file SweepTimeloop.cpp
 //! \ingroup timeloop
 //! \author Martin Bauer <martin.bauer@fau.de>
+//! \author Christoph Schwarzmeier <christoph.schwarzmeier@fau.de>
 //
 //======================================================================================================================
 
@@ -47,16 +48,16 @@ void SweepTimeloop::doTimeStep(const Set<SUID> &selectors)
       // Loop over all blocks
       for( BlockStorage::iterator bi = blockStorage_.begin(); bi != blockStorage_.end(); ++bi )
       {
+         // ensure that at least one sweep has been registered (regardless of its selector)
          if( s.sweep.empty() )
          {
             WALBERLA_ABORT("Selecting Sweep " << sweepIt->first << ": " <<
                            "No sweep has been registered! Did you only register a BeforeFunction or AfterFunction?" );
          }
 
-         Sweep selectedSweep;
-         size_t numSweeps = s.sweep.get(selectedSweep, selectors + bi->getState());
+         // ensure that exactly one sweep has been registered that matches the specified selectors
+         size_t numSweeps = s.sweep.getNumberOfMatching(selectors + bi->getState());
 
-         // ensure that no more than one sweep has been added to a single SweepAdder object
          if (numSweeps == size_t(0)) {
             continue;
          } else {
@@ -66,6 +67,8 @@ void SweepTimeloop::doTimeStep(const Set<SUID> &selectors)
             }
          }
 
+         Sweep * selectedSweep = s.sweep.getUnique( selectors + bi->getState() );
+
          WALBERLA_LOG_PROGRESS_SECTION()
          {
             std::string sweepName;
@@ -73,7 +76,7 @@ void SweepTimeloop::doTimeStep(const Set<SUID> &selectors)
             WALBERLA_LOG_PROGRESS("Running sweep \"" << sweepName << "\" on block " << bi->getId() );
          }
 
-         (selectedSweep.function_)( bi.get() );
+         (selectedSweep->function_)( bi.get() );
       }
 
       // select and execute after functions
@@ -114,11 +117,16 @@ void SweepTimeloop::doTimeStep(const Set<SUID> &selectors, WcTimingPool &timing)
 
       for( BlockStorage::iterator bi = blockStorage_.begin(); bi != blockStorage_.end(); ++bi )
       {
-         Sweep selectedSweep;
-         std::string sweepName;
-         size_t numSweeps = s.sweep.get(selectedSweep, sweepName, selectors + bi->getState());
+         // ensure that at least one sweep has been registered (regardless of its selector)
+         if( s.sweep.empty() )
+         {
+            WALBERLA_ABORT("Selecting Sweep " << sweepIt->first << ": " <<
+                           "No sweep has been registered! Did you only register a BeforeFunction or AfterFunction?" );
+         }
+
+         // ensure that exactly one sweep has been registered that matches the specified selectors
+         size_t numSweeps = s.sweep.getNumberOfMatching(selectors + bi->getState());
 
-         // ensure that no more than one sweep has been added to a single SweepAdder object
          if (numSweeps == size_t(0)) {
             continue;
          } else {
@@ -128,11 +136,14 @@ void SweepTimeloop::doTimeStep(const Set<SUID> &selectors, WcTimingPool &timing)
             }
          }
 
+         std::string sweepName;
+         Sweep * selectedSweep = s.sweep.getUnique( selectors + bi->getState(), sweepName );
+
          WALBERLA_LOG_PROGRESS("Running sweep \"" << sweepName << "\" on block " << bi->getId() );
 
          // loop over all blocks
          timing[sweepName].start();
-         (selectedSweep.function_)( bi.get() );
+         (selectedSweep->function_)( bi.get() );
          timing[sweepName].end();
       }
 

tests/timeloop/CMakeLists.txt

@@ -4,6 +4,20 @@
 #
 ###################################################################################################
 
+waLBerla_compile_test( FILES    MultipleSweepFailTest.cpp
+                       DEPENDS  blockforest )
+waLBerla_execute_test( NAME     MultipleSweepFailTest )
+set_property(          TEST     MultipleSweepFailTest
+                       PROPERTY WILL_FAIL TRUE )
 
-waLBerla_compile_test( FILES TimeloopAndSweepRegister.cpp DEPENDS field blockforest )
-waLBerla_execute_test(NAME TimeloopAndSweepRegister )
+waLBerla_compile_test( FILES    MultipleSweepTest.cpp
+                       DEPENDS  blockforest )
+waLBerla_execute_test( NAME     MultipleSweepTest )
+
+waLBerla_compile_test( FILES    TimeloopSweepManagementTest.cpp
+                       DEPENDS  blockforest )
+waLBerla_execute_test( NAME     TimeloopSweepManagementTest )
+
+waLBerla_compile_test( FILES    TimeloopAndSweepRegisterTest.cpp
+                       DEPENDS  blockforest field )
+waLBerla_execute_test( NAME     TimeloopAndSweepRegisterTest )
\ No newline at end of file

tests/timeloop/MultipleSweepFailTest.cpp

+//======================================================================================================================
+//
+//  This file is part of waLBerla. waLBerla is free software: you can
+//  redistribute it and/or modify it under the terms of the GNU General Public
+//  License as published by the Free Software Foundation, either version 3 of
+//  the License, or (at your option) any later version.
+//
+//  waLBerla is distributed in the hope that it will be useful, but WITHOUT
+//  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+//  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+//  for more details.
+//
+//  You should have received a copy of the GNU General Public License along
+//  with waLBerla (see COPYING.txt). If not, see <http://www.gnu.org/licenses/>.
+//
+//! \file MultipleSweepFailTest.cpp
+//! \ingroup timeloop
+//! \author Markus Holzer <markus.holzer@fau.de>
+//! \author Christoph Schwarzmeier <christoph.schwarzmeier@fau.de>
+//! \brief Test if multiple sweeps (with the same selector) can not be added to the timeloop at once.
+//!
+//! THIS TEST MUST FAIL!
+//
+//======================================================================================================================
+
+#include "blockforest/Initialization.h"
+
+#include "core/DataTypes.h"
+#include "core/debug/TestSubsystem.h"
+#include "core/mpi/Environment.h"
+
+#include "timeloop/SweepTimeloop.h"
+
+namespace walberla
+{
+namespace timeloop
+{
+namespace MultipleSweepFailTest
+{
+
+int main(int argc, char** argv)
+{
+   debug::enterTestMode();
+   mpi::Environment env(argc, argv);
+
+   const std::shared_ptr< StructuredBlockForest > blockForest = blockforest::createUniformBlockGrid(
+      uint_c(1), uint_c(1), uint_c(1), uint_c(1), uint_c(1), uint_c(1), real_c(1), false, false, false, false);
+
+   SweepTimeloop timeloop(blockForest, uint_c(1));
+
+   // empty sweep that does nothing
+   const auto emptySweep = [](IBlock*) {};
+
+   // this must fail, as two sweeps are added at once with the same selectors (default selectors: Set<SUID>::emptySet())
+   timeloop.add() << Sweep(emptySweep, "Sweep 1") << Sweep(emptySweep, "Sweep 2");
+   timeloop.singleStep();
+
+   return EXIT_SUCCESS;
+}
+} // namespace MultipleSweepFailTest
+} // namespace timeloop
+} // namespace walberla
+
+int main(int argc, char** argv) { return walberla::timeloop::MultipleSweepFailTest::main(argc, argv); }

tests/timeloop/MultipleSweepTest.cpp

+//======================================================================================================================
+//
+//  This file is part of waLBerla. waLBerla is free software: you can
+//  redistribute it and/or modify it under the terms of the GNU General Public
+//  License as published by the Free Software Foundation, either version 3 of
+//  the License, or (at your option) any later version.
+//
+//  waLBerla is distributed in the hope that it will be useful, but WITHOUT
+//  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+//  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+//  for more details.
+//
+//  You should have received a copy of the GNU General Public License along
+//  with waLBerla (see COPYING.txt). If not, see <http://www.gnu.org/licenses/>.
+//
+//! \file MultipleSweepTest.cpp
+//! \ingroup timeloop
+//! \author Markus Holzer <markus.holzer@fau.de>
+//! \author Christoph Schwarzmeier <christoph.schwarzmeier@fau.de>
+//! \brief Test if multiple sweeps (with different selectors) can be added to the timeloop at once.
+//
+//======================================================================================================================
+
+#include "blockforest/Initialization.h"
+
+#include "core/DataTypes.h"
+#include "core/debug/TestSubsystem.h"
+#include "core/mpi/Environment.h"
+
+#include "timeloop/SweepTimeloop.h"
+
+namespace walberla
+{
+namespace timeloop
+{
+namespace MultipleSweepTest
+{
+
+int main(int argc, char** argv)
+{
+   debug::enterTestMode();
+   mpi::Environment env(argc, argv);
+
+   const std::shared_ptr< StructuredBlockForest > blockForest = blockforest::createUniformBlockGrid(
+      uint_c(1), uint_c(1), uint_c(1), uint_c(1), uint_c(1), uint_c(1), real_c(1), false, false, false, false);
+
+   SweepTimeloop timeloop(blockForest, uint_c(1));
+
+   // empty sweep that does nothing
+   const auto emptySweep = [](IBlock*) {};
+
+   const SUID selector1("selector1");
+   const SUID selector2("selector2");
+   const SUID selector3("selector3");
+   const SUID selector4("selector4");
+
+   // this must not fail, as two sweeps with the different (required) selectors are added
+   timeloop.add() << Sweep(emptySweep, "Sweep 1", selector1, Set< SUID >::emptySet())
+                  << Sweep(emptySweep, "Sweep 2", selector2, Set< SUID >::emptySet());
+   timeloop.add() << Sweep(emptySweep, "Sweep 3", selector3, selector4)
+                  << Sweep(emptySweep, "Sweep 4", selector4, selector3);
+
+   timeloop.singleStep();
+
+   return EXIT_SUCCESS;
+}
+} // namespace MultipleSweepTest
+} // namespace timeloop
+} // namespace walberla
+
+int main(int argc, char** argv) { return walberla::timeloop::MultipleSweepTest::main(argc, argv); }

tests/timeloop/TimeloopAndSweepRegister.cpp → tests/timeloop/TimeloopAndSweepRegisterTest.cpp

@@ -13,10 +13,11 @@
 //  You should have received a copy of the GNU General Public License along
 //  with waLBerla (see COPYING.txt). If not, see <http://www.gnu.org/licenses/>.
 //
-//! \file TimeloopAndSweepRegister.cpp
+//! \file TimeloopAndSweepRegisterTest.cpp
 //! \ingroup timeloop
 //! \author Markus Holzer <markus.holzer@fau.de>
-//! \brief test cases that test the registering of Sweeps at timeloop
+//! \author Christoph Schwarzmeier <christoph.schwarzmeier@fau.de>
+//! \brief Test if registering and execution of sweeps in a timeloop with different selectors works correctly.
 //
 //======================================================================================================================
 
@@ -30,17 +31,21 @@
 
 #include "timeloop/SweepTimeloop.h"
 
-#include <string>
 #include <vector>
 
-using namespace walberla;
+namespace walberla
+{
+namespace timeloop
+{
+namespace TimeloopAndSweepRegisterTest
+{
 
 using Field_T = Field< uint_t, 1 >;
 
 auto FieldAdder = [](IBlock* const block, StructuredBlockStorage* const storage) {
    return new Field_T(storage->getNumberOfXCells(*block), storage->getNumberOfYCells(*block),
-                      storage->getNumberOfZCells(*block), uint_t(0.0), field::fzyx,
-                      make_shared< field::AllocateAligned< uint_t, 64 > >());
+                      storage->getNumberOfZCells(*block), uint_t(0), field::fzyx,
+                      make_shared< field::AllocateAligned< uint_t, uint_t(64) > >());
 };
 
 class Sweep1
@@ -48,82 +53,86 @@ class Sweep1
  public:
    Sweep1(BlockDataID fieldID) : fieldID_(fieldID) {}
 
-   void operator()(IBlock* block)
+   void operator()(IBlock* const block)
    {
-      auto field = block->getData< Field_T >(fieldID_);
+      Field_T* field = block->getData< Field_T >(fieldID_);
 
-      for (auto iter = field->begin(); iter != field->end(); ++iter)
-         *iter += 1;
+      WALBERLA_FOR_ALL_CELLS(fieldIt, field, { *fieldIt += uint_c(1); }) // WALBERLA_FOR_ALL_CELLS
    }
 
  private:
    BlockDataID fieldID_;
-};
+}; // class Sweep1
 
 class Sweep2
 {
  public:
    Sweep2(BlockDataID fieldID) : fieldID_(fieldID) {}
 
-   void operator()(IBlock* block)
+   void operator()(IBlock* const block)
    {
-      auto field = block->getData< Field_T >(fieldID_);
-
-      for (auto iter = field->begin(); iter != field->end(); ++iter)
-         *iter += 2;
+      Field_T* field = block->getData< Field_T >(fieldID_);
+      WALBERLA_FOR_ALL_CELLS(fieldIt, field, { *fieldIt += uint_c(2); }) // WALBERLA_FOR_ALL_CELLS
    }
 
  private:
    BlockDataID fieldID_;
-};
+}; // class Sweep2
 
 int main(int argc, char** argv)
 {
    debug::enterTestMode();
    mpi::Environment env(argc, argv);
 
-   std::vector<std::string> expectedSequence;
-   std::vector<std::string> sequence;
+   std::vector< std::string > expectedSequence;
+   std::vector< std::string > sequence;
 
-   SUID sweepSelect1("Sweep1");
-   SUID sweepSelect2("Sweep2");
+   const SUID sweepSelect1("Sweep1");
+   const SUID sweepSelect2("Sweep2");
 
-   shared_ptr< StructuredBlockForest > blocks = blockforest::createUniformBlockGrid(
+   const shared_ptr< StructuredBlockForest > blockForest = blockforest::createUniformBlockGrid(
       uint_c(4), uint_c(2), uint_c(2), uint_c(10), uint_c(10), uint_c(10), real_c(1), false, false, false, false);
 
-   BlockDataID fieldID = blocks->addStructuredBlockData< Field_T >(FieldAdder, "Test Field");
+   const BlockDataID fieldID = blockForest->addStructuredBlockData< Field_T >(FieldAdder, "Test Field");
 
-   for (auto& block : *blocks)
+   for (auto& block : *blockForest)
    {
-      if (block.getAABB().min()[0] < 20)
-         block.setState(sweepSelect1);
+      if (block.getAABB().min()[0] < real_c(20)) { block.setState(sweepSelect1); }
       else
+      {
          block.setState(sweepSelect2);
+      }
    }
 
-   uint_t timesteps = 10;
-   SweepTimeloop timeloop(blocks->getBlockStorage(), timesteps);
+   const uint_t timesteps = uint_c(10);
+   SweepTimeloop timeloop(blockForest, timesteps);
 
    timeloop.add() << Sweep(Sweep1(fieldID), "Sweep 1", sweepSelect1, sweepSelect2);
    timeloop.add() << Sweep(Sweep2(fieldID), "Sweep 2", sweepSelect2, sweepSelect1);
 
-   WcTimingPool timingPool;
+   timeloop.run();
 
-   timeloop.run(timingPool);
-   for (auto& block : *blocks)
+   for (const auto& block : *blockForest)
    {
-      auto field = block.getData< Field_T >(fieldID);
-      if (block.getAABB().min()[0] < 20)
+      const Field_T* field = block.getData< Field_T >(fieldID);
+
+      if (block.getAABB().min()[0] < real_c(20))
       {
-         for (auto iter = field->begin(); iter != field->end(); ++iter)
-            WALBERLA_CHECK_EQUAL(*iter, timesteps)
+         WALBERLA_FOR_ALL_CELLS(fieldIt, field,
+                                { WALBERLA_CHECK_EQUAL(*fieldIt, timesteps); }) // WALBERLA_FOR_ALL_CELLS
       }
       else
       {
-         for (auto iter = field->begin(); iter != field->end(); ++iter)
-            WALBERLA_CHECK_EQUAL(*iter, timesteps * 2)
+         WALBERLA_FOR_ALL_CELLS(fieldIt, field,
+                                { WALBERLA_CHECK_EQUAL(*fieldIt, timesteps * uint_c(2)); }) // WALBERLA_FOR_ALL_CELLS
       }
    }
-   
+
    return EXIT_SUCCESS;
 }
+
+} // namespace TimeloopAndSweepRegisterTest
+} // namespace timeloop
+} // namespace walberla
+
+int main(int argc, char** argv) { return walberla::timeloop::TimeloopAndSweepRegisterTest::main(argc, argv); }
\ No newline at end of file

tests/timeloop/TimeloopSweepManagementTest.cpp

+//======================================================================================================================
+//
+//  This file is part of waLBerla. waLBerla is free software: you can
+//  redistribute it and/or modify it under the terms of the GNU General Public
+//  License as published by the Free Software Foundation, either version 3 of
+//  the License, or (at your option) any later version.
+//
+//  waLBerla is distributed in the hope that it will be useful, but WITHOUT
+//  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+//  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+//  for more details.
+//
+//  You should have received a copy of the GNU General Public License along
+//  with waLBerla (see COPYING.txt). If not, see <http://www.gnu.org/licenses/>.
+//
+//! \file TimeloopSweepManagementTest.cpp
+//! \ingroup timeloop
+//! \author Markus Holzer <markus.holzer@fau.de>
+//! \author Christoph Schwarzmeier <christoph.schwarzmeier@fau.de>
+//! \brief Test if a sweep in the timeloop works on its members rather can re-initializing them in every time step.
+//
+//======================================================================================================================
+
+#include "blockforest/Initialization.h"
+
+#include "core/DataTypes.h"
+#include "core/Environment.h"
+#include "core/debug/TestSubsystem.h"
+
+#include "timeloop/SweepTimeloop.h"
+
+namespace walberla
+{
+namespace timeloop
+{
+namespace TimeloopSweepManagementTest
+{
+
+class CounterSweep
+{
+ public:
+   CounterSweep(const std::shared_ptr< uint_t >& externalCounter)
+      : externalCounter_(externalCounter), internalCounter_(uint_c(0))
+   {}
+
+   void operator()(IBlock*)
+   {
+      ++(*externalCounter_);
+      ++internalCounter_;
+
+      WALBERLA_CHECK_EQUAL(*externalCounter_, internalCounter_);
+   }
+
+ private:
+   std::shared_ptr< uint_t > externalCounter_;
+   uint_t internalCounter_;
+
+}; // class CounterSweep
+
+int main(int argc, char** argv)
+{
+   debug::enterTestMode();
+   mpi::Environment env(argc, argv);
+
+   shared_ptr< StructuredBlockForest > blockForest =
+      blockforest::createUniformBlockGrid(uint_c(1), uint_c(1), uint_c(1), uint_c(1), uint_c(1), uint_c(1), real_c(1));
+
+   SweepTimeloop timeloop(blockForest, uint_c(10));
+
+   const std::shared_ptr< uint_t > counter = std::make_shared< uint_t >(uint_c(0));
+
+   auto TestSweep = Sweep(CounterSweep(counter), "Counter sweep");
+   timeloop.add() << TestSweep;
+   timeloop.run();
+
+   return EXIT_SUCCESS;
+}
+} // namespace TimeloopSweepManagementTest
+} // namespace timeloop
+} // namespace walberla
+
+int main(int argc, char** argv) { return walberla::timeloop::TimeloopSweepManagementTest::main(argc, argv); }