Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core][MPI] Adding SpatialSearchResultContainer and SpatialSearchResultContainerMap (v.2.0) #11416

Closed

Conversation

loumalouomega
Copy link
Member

@loumalouomega loumalouomega commented Jul 21, 2023

📝 Description

#11205 + addressed comments

This PR introduces a new class named SpatialSearchResultContainer and SpatialSearchResultContainerMap. The class provides storage and operations related to spatial search results. This is a transition PR to finally give support for MPI search using GeometricalObjectBins.

The SpatialSearchResultContainer class stores:

  1. Local pointers of results (in a PointerVectorSet)
  2. Global pointers (empty if not synchronized)
  3. GlobalPointerCommunicator pointer (to synchronize the global pointers)

The class includes functions such as, SynchronizeAll, HasResult, GetResultIndices, AddResult... The interesting part is that integrates a global pointer communicator, so it is possible to calculate anything passing a functor. The most common operations required are already implemented: GetResultShapeFunctions, GetResultIndices and GetResultCoordinates,

The SynchronizeAll method synchronizes the container between partitions. The AddResult method adds a result to the local pointers. There is a push_back which does the same.

The SpatialSearchResultContainerMap class stores an unordered map of SpatialSearchResultContainer, using an integer as hash. The hash is computed from the point coordinates pointer address (comment from #11205 by @pooyan-dadvand ) which we are computing the results.

This commit seems to be a significant enhancement, providing a new means of handling and manipulating spatial search results in the Kratos library.

We need to derive SpatialSearchResult from indexed object in order to be able to use PointerVectorSet in #11416, see

using LocalResultsVector = PointerVectorSet<SpatialSearchResultType,
, which is required by the GlobalPointerUtilities in order to keep changes to the minimum. Otherwise I will need to add manually the vector of SpatialSearchResult the GlobalPointerUtilities in pourpose, with the corresponding debugging, problematics and error prone due to code duplication. This solution is therefore much cleaner.

🆕 Changelog

@loumalouomega loumalouomega added Enhancement Kratos Core Parallel-MPI Distributed memory parallelism for HPC / clusters Transition labels Jul 21, 2023
@loumalouomega loumalouomega requested review from a team as code owners July 21, 2023 21:11
@loumalouomega
Copy link
Member Author

Ping @pooyan-dadvand

/***********************************************************************************/

template <class TObjectType>
std::vector<Vector> SpatialSearchResultContainer<TObjectType>::GetResultShapeFunctions(const array_1d<double, 3>& rPoint)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@pooyan-dadvand pooyan-dadvand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed with @roigcarlo and @RiccardoRossi and @jcotela we will pass the new interface to a new meta search class for MPI and leave the one by one interface only for local searches as before

@@ -54,6 +56,8 @@ class KRATOS_API(KRATOS_CORE) GeometricalObjectsBins
/// The type of geometrical object to be stored in the bins
using CellType = std::vector<GeometricalObject*>;
using ResultType = SpatialSearchResult<GeometricalObject>;
using ResultTypeContainer = SpatialSearchResultContainer<GeometricalObject>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResultContainerType

@@ -54,6 +56,8 @@ class KRATOS_API(KRATOS_CORE) GeometricalObjectsBins
/// The type of geometrical object to be stored in the bins
using CellType = std::vector<GeometricalObject*>;
using ResultType = SpatialSearchResult<GeometricalObject>;
using ResultTypeContainer = SpatialSearchResultContainer<GeometricalObject>;
using ResultTypeContainerMap = SpatialSearchResultContainerMap<GeometricalObject>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResultContainerMapType

}
mBoundingBox.Extend(mTolerance);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why moving inside?


// Synchronize if needed
if (SyncronizeResults) {
rResults.SynchronizeAll(ParallelEnvironment::GetDefaultDataCommunicator());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure using only the default communicator is not very limiting

@loumalouomega
Copy link
Member Author

Closing as included in #11719

@loumalouomega loumalouomega deleted the core/mpi/spatial-search-container-new-proposal branch November 2, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Kratos Core Parallel-MPI Distributed memory parallelism for HPC / clusters Transition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants