[Core][PVS] Add PointerVectorSet::MutablePass to allow push_back safely.

applications/HDF5Application/custom_io/hdf5_connectivities_data.cpp

@@ -79,6 +79,7 @@ void ConnectivitiesData::CreateEntities(NodesContainerType& rNodes,
     KRATOS_ERROR_IF(geometry_size != mConnectivities.size2())
         << "Non-matching geometry and connectivity sizes." << std::endl;
     rElements.reserve(rElements.size() + num_new_elems);
+    auto mutable_pass = rElements.GetMutablePass();
     ElementType::NodesArrayType nodes(geometry_size);
 
     for (unsigned i = 0; i < num_new_elems; ++i)
@@ -90,7 +91,7 @@ void ConnectivitiesData::CreateEntities(NodesContainerType& rNodes,
         }
         ElementType::Pointer p_elem =
             r_elem.Create(mIds[i], nodes, rProperties(mPropertiesIds[i]));
-        rElements.push_back(p_elem);
+        mutable_pass.push_back(p_elem);
     }
     KRATOS_CATCH("");
 }
@@ -108,6 +109,7 @@ void ConnectivitiesData::CreateEntities(NodesContainerType& rNodes,
     KRATOS_ERROR_IF(geometry_size != mConnectivities.size2())
         << "Non-matching geometry and connectivity sizes." << std::endl;
     rConditions.reserve(rConditions.size() + num_new_conds);
+    auto mutable_pass = rConditions.GetMutablePass();
     ConditionType::NodesArrayType nodes(geometry_size);
 
     for (unsigned i = 0; i < num_new_conds; ++i)
@@ -119,7 +121,7 @@ void ConnectivitiesData::CreateEntities(NodesContainerType& rNodes,
         }
         Condition::Pointer p_cond =
             r_cond.Create(mIds[i], nodes, rProperties(mPropertiesIds[i]));
-        rConditions.push_back(p_cond);
+        mutable_pass.push_back(p_cond);
     }
     KRATOS_CATCH("");
 }

applications/HDF5Application/custom_io/hdf5_points_data.cpp

@@ -31,12 +31,13 @@ void PointsData::CreateNodes(NodesContainerType& rNodes)
     KRATOS_TRY;
     const unsigned num_new_nodes = mIds.size();
     rNodes.reserve(rNodes.size() + num_new_nodes);
+    auto mutable_pass = rNodes.GetMutablePass();
     for (unsigned i = 0; i < num_new_nodes; ++i)
     {
         const array_1d<double, 3>& r_coord = mCoords[i];
         NodeType::Pointer p_node = Kratos::make_intrusive<NodeType>(
             mIds[i], r_coord[0], r_coord[1], r_coord[2]);
-        rNodes.push_back(p_node);
+        mutable_pass.push_back(p_node);
     }
     KRATOS_CATCH("");
 }

applications/HDF5Application/tests/test_hdf5_model_part_io_mpi.py

@@ -203,8 +203,6 @@ def test_HDF5ModelPartIO(self):
                 self.assertEqual(read_node.Z, write_node.Z)
             # Check elements
             self.assertEqual(read_model_part.NumberOfElements(), write_model_part.NumberOfElements())
-            first_elem_id = next(iter(read_model_part.Elements)).Id
-            read_model_part.GetElement(first_elem_id) # Force a sort since order is mixed by openmp.
             for read_elem, write_elem in zip(read_model_part.Elements, write_model_part.Elements):
                 self.assertEqual(read_elem.Id, write_elem.Id)
                 self.assertEqual(read_elem.Properties.Id, write_elem.Properties.Id)
@@ -213,8 +211,6 @@ def test_HDF5ModelPartIO(self):
                     self.assertEqual(read_elem_node.Id, write_elem_node.Id)
             # Check conditions
             self.assertEqual(read_model_part.NumberOfConditions(), write_model_part.NumberOfConditions())
-            first_cond_id = next(iter(read_model_part.Conditions)).Id
-            read_model_part.GetCondition(first_cond_id) # Force a sort since order is mixed by openmp.
             for read_cond, write_cond in zip(read_model_part.Conditions, write_model_part.Conditions):
                 self.assertEqual(read_cond.Id, write_cond.Id)
                 self.assertEqual(read_cond.Properties.Id, write_cond.Properties.Id)

kratos/containers/pointer_vector_set.h

@@ -20,6 +20,7 @@
 #include <sstream>
 #include <cstddef>
 #include <utility>
+#include <type_traits>
 
 // External includes
 #include <boost/iterator/indirect_iterator.hpp>
@@ -28,6 +29,7 @@
 #include "includes/define.h"
 #include "includes/serializer.h"
 #include "containers/key_generator.h"
+#include "utilities/parallel_utilities.h"
 
 namespace Kratos
 {
@@ -107,6 +109,89 @@ class PointerVectorSet final
     using ptr_const_reverse_iterator = typename TContainerType::const_reverse_iterator;
     using difference_type = typename TContainerType::difference_type;
 
+    ///@}
+    ///@name Class definitions
+    ///@{
+
+    /// @brief Unrestricted accessor for the pointer vector set
+    /// @details This class provides unrestricted access the underlying std::vector of the pointer vector
+    ///          set which may make the pointer vector set unsorted. Hence, once an object of this is created, the
+    ///          methods which utilize the sorted property of pointer vector set are frozen. At the destruction of this class,
+    ///          pointer vector set is sorted and made unique, and the frozen methods are released.
+    /// @param
+    /// @return
+    class MutablePass
+    {
+        ///@name Life cycle
+        ///@{
+
+        // Constructor
+        MutablePass(PointerVectorSet& rContainer)
+            : mrContainer(rContainer)
+        {
+            KRATOS_CRITICAL_SECTION
+            KRATOS_ERROR_IF(mrContainer.mHasMutablePass)
+                << "A mutable pass is active. Hence, creation of an another MutablePass is prohibited.";
+            mrContainer.mHasMutablePass = true;
+        }
+
+        ///@}
+
+    public:
+        ///@name Life cycle
+        ///@{
+
+        MutablePass(const MutablePass& rOther) = delete;
+
+        MutablePass(MutablePass&& rOther) noexcept = default;
+
+        // Destructor
+        ~MutablePass()
+        {
+            // Sort the PointerVectorSet data
+            std::sort(mrContainer.mData.begin(), mrContainer.mData.end(), CompareKey());
+
+            // Make the entities unique
+            auto new_end_it = std::unique(mrContainer.mData.begin(), mrContainer.mData.end(), EqualKeyTo());
+
+            // remove the duplicated entities.
+            mrContainer.mData.erase(new_end_it,  mrContainer.mData.end());
+
+            // unfreeze the methods which dependent on the sorted state of the PointerVectorSet
+            mrContainer.mHasMutablePass = false;
+        }
+
+        ///@name Public operators
+        ///@{
+
+        MutablePass& operator=(const MutablePass& rOther) = delete;
+
+        ///@}
+        ///@name Public methods
+        ///@{
+
+        void push_back(TPointerType pValue)
+        {
+            mrContainer.mData.push_back(pValue);
+        }
+
+        ///@}
+
+    private:
+        ///@name Member variables
+        ///@{
+
+        PointerVectorSet& mrContainer;
+
+        ///@}
+        ///@name Friend classes
+        ///@{
+
+        friend class PointerVectorSet;
+
+        ///@}
+    };
+
     ///@}
     ///@name Life Cycle
     ///@{
@@ -178,29 +263,15 @@ class PointerVectorSet final
      */
     TDataType& operator[](const key_type& Key)
     {
-        ptr_iterator sorted_part_end;
-
-        if (mData.size() - mSortedPartSize >= mMaxBufferSize) {
-            Sort();
-            sorted_part_end = mData.end();
+        KRATOS_ERROR_IF(mHasMutablePass)
+            << "A mutable pass is active. Hence, use of PointerVectorSet::operator[] is prohibited.";
+        ptr_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey()));
+        if (i != mData.end() && EqualKeyTo(Key)(*i)) {
+            return **i;
         } else {
-            sorted_part_end = mData.begin() + mSortedPartSize;
-        }
-
-        ptr_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey()));
-        if (i == sorted_part_end) {
-            mSortedPartSize++;
-            return **mData.insert(sorted_part_end, TPointerType(new TDataType(Key)));
+            static_assert(!std::is_pointer_v<TPointerType>, "Raw pointers are not supported.");
+            return **mData.insert(i, TPointerType(new TDataType(Key)));
         }
-
-        if (!EqualKeyTo(Key)(*i)) {
-            if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end()) {
-                mData.push_back(TPointerType(new TDataType(Key)));
-                return **(mData.end() - 1);
-            }
-        }
-
-        return **i;
     }
 
     /**
@@ -213,27 +284,15 @@ class PointerVectorSet final
      */
     pointer& operator()(const key_type& Key)
     {
-        ptr_iterator sorted_part_end;
-
-        if (mData.size() - mSortedPartSize >= mMaxBufferSize) {
-            Sort();
-            sorted_part_end = mData.end();
-        } else
-            sorted_part_end = mData.begin() + mSortedPartSize;
-
-        ptr_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey()));
-        if (i == sorted_part_end) {
-            mSortedPartSize++;
-            return *mData.insert(sorted_part_end, TPointerType(new TDataType(Key)));
+        KRATOS_ERROR_IF(mHasMutablePass)
+            << "A mutable pass is active. Hence, use of PointerVectorSet::operator() is prohibited.";
+        ptr_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey()));
+        if (i != mData.end() && EqualKeyTo(Key)(*i)) {
+            return *i;
+        } else {
+            static_assert(!std::is_pointer_v<TPointerType>, "Raw pointers are not supported.");
+            return *mData.insert(i, TPointerType(new TDataType(Key)));
         }
-
-        if (!EqualKeyTo(Key)(*i))
-            if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end()) {
-                mData.push_back(TPointerType(new TDataType(Key)));
-                return *(mData.end() - 1);
-            }
-
-        return *i;
     }
 
     /**
@@ -533,6 +592,7 @@ class PointerVectorSet final
     {
         std::swap(mSortedPartSize, rOther.mSortedPartSize);
         std::swap(mMaxBufferSize, rOther.mMaxBufferSize);
+        std::swap(mHasMutablePass, rOther.mHasMutablePass);
         mData.swap(rOther.mData);
     }
 
@@ -568,6 +628,8 @@ class PointerVectorSet final
      */
     iterator insert(const TPointerType& value)
     {
+        KRATOS_ERROR_IF(mHasMutablePass)
+            << "A mutable pass is active. Hence, use of PointerVectorSet::insert(value) is prohibited.";
         auto itr_pos = std::lower_bound(mData.begin(), mData.end(), KeyOf(*value), CompareKey());
         if (itr_pos == mData.end()) {
             // the position to insert is at the end.
@@ -596,6 +658,8 @@ class PointerVectorSet final
      */
     iterator insert(const_iterator position_hint, const TPointerType& value)
     {
+        KRATOS_ERROR_IF(mHasMutablePass)
+            << "A mutable pass is active. Hence, use of PointerVectorSet::insert(position_hint, value) is prohibited.";
         if (empty()) {
             // the dataset is empty. So use push back.
             mData.push_back(value);
@@ -741,20 +805,14 @@ class PointerVectorSet final
      */
     iterator find(const key_type& Key)
     {
-        ptr_iterator sorted_part_end;
-
-        if (mData.size() - mSortedPartSize >= mMaxBufferSize) {
-            Sort();
-            sorted_part_end = mData.end();
-        } else
-            sorted_part_end = mData.begin() + mSortedPartSize;
-
-        ptr_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey()));
-        if (i == sorted_part_end || (!EqualKeyTo(Key)(*i)))
-            if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end())
-                return mData.end();
-
-        return i;
+        KRATOS_ERROR_IF(mHasMutablePass)
+            << "A mutable pass is active. Hence, use of PointerVectorSet::find(key) is prohibited.";
+        ptr_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey()));
+        if (i != mData.end() && EqualKeyTo(Key)(*i)) {
+            return i;
+        } else {
+            return mData.end();
+        }
     }
 
     /**
@@ -767,14 +825,14 @@ class PointerVectorSet final
      */
     const_iterator find(const key_type& Key) const
     {
-        ptr_const_iterator sorted_part_end(mData.begin() + mSortedPartSize);
-
-        ptr_const_iterator i(std::lower_bound(mData.begin(), sorted_part_end, Key, CompareKey()));
-        if (i == sorted_part_end || (!EqualKeyTo(Key)(*i)))
-            if ((i = std::find_if(sorted_part_end, mData.end(), EqualKeyTo(Key))) == mData.end())
-                return mData.end();
-
-        return const_iterator(i);
+        KRATOS_ERROR_IF(mHasMutablePass)
+            << "A mutable pass is active. Hence, use of PointerVectorSet::find(key) is prohibited.";
+        ptr_const_iterator i(std::lower_bound(mData.begin(), mData.end(), Key, CompareKey()));
+        if (i != mData.end() && EqualKeyTo(Key)(*i)) {
+            return const_iterator(i);
+        } else {
+            return mData.end();
+        }
     }
 
     /**
@@ -838,6 +896,11 @@ class PointerVectorSet final
     ///@name Access
     ///@{
 
+    MutablePass GetMutablePass()
+    {
+        return MutablePass(*this);
+    }
+
     /** Gives a reference to underly normal container. */
     TContainerType& GetContainer()
     {
@@ -1062,6 +1125,8 @@ class PointerVectorSet final
     /// The maximum buffer size for data storage.
     size_type mMaxBufferSize;
 
+    bool mHasMutablePass = false;
+
     ///@}
     ///@name Private Operators
     ///@{
@@ -1083,6 +1148,9 @@ class PointerVectorSet final
                 mData.push_back(GetPointer(it));
             }
         } else {
+            KRATOS_ERROR_IF(mHasMutablePass)
+                << "A mutable pass is active. Hence, use of PointerVectorSet::insert(first, last) is prohibited.";            
+
             if (KeyOf(GetReference(first)) > KeyOf(*(mData.back()))) {
                 // all are pointing to the end of the vector, hence pushing back.
                 mData.reserve(mData.size() + std::distance(first, last));
@@ -1213,7 +1281,7 @@ class PointerVectorSet final
     }
 
     ///@}
-    ///@name Serialization
+    ///@name Friend classes
     ///@{
 
     /**
@@ -1222,6 +1290,19 @@ class PointerVectorSet final
      */
     friend class Serializer;
 
+    /**
+     * @class MutablePass
+     * @brief A friend class responsible for giving a pass to mutate which may destroy the sorted state
+     *        of PointerVectorSet. All methods which relies on the sorted state will be frozen when this is
+     *        active, and they will be unfrozen and PointerVectorSet will be sorted at the destruction on the
+     *        MutablePass.
+     */
+    friend class MutablePass;
+
+    ///@}
+    ///@name Serialization
+    ///@{
+
     /**
      * @brief Extract the object's state and uses the Serializer to store it.
      * @param rSerializer Serializer instance to be used for saving.