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

fix: Fix misuse of CommID #3402

Merged
merged 1 commit into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/coreComponents/mesh/mpiCommunications/CommID.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,16 @@ class CommID
*/
~CommID();

/// default copy constructor
CommID( CommID const & ) = default;

/**
* Move constructor
* @param src The source to move data from.
*/
CommID( CommID && src );

/// deleted default copy constructor
CommID( CommID const & ) = delete;

/// deleted copy assignment operator
CommID & operator=( CommID const & ) = delete;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void CommunicationTools::assignGlobalIndices( ObjectManagerBase & manager,

integer const numNeighbors = LvArray::integerConversion< integer >( neighbors.size() );

MPI_iCommData commData( getCommID() );
MPI_iCommData commData;
commData.resize( numNeighbors );

array1d< int > receiveBufferSizes( numNeighbors );
Expand Down Expand Up @@ -340,11 +340,11 @@ CommunicationTools::assignNewGlobalIndices( ElementRegionManager & elementManage
* @return The data received from all the @p neighbors. Data at index @p i coming from neighbor at index @p i in the list of @p neighbors.
*/
template< class DATA_PROVIDER >
array1d< array1d< globalIndex > > exchange( int commId,
std::vector< NeighborCommunicator > & neighbors,
array1d< array1d< globalIndex > > exchange( std::vector< NeighborCommunicator > & neighbors,
DATA_PROVIDER const & data )
{
MPI_iCommData commData( commId );
MPI_iCommData commData;
int commId = commData.commID();
integer const numNeighbors = LvArray::integerConversion< integer >( neighbors.size() );
commData.resize( numNeighbors );
for( integer i = 0; i < numNeighbors; ++i )
Expand Down Expand Up @@ -391,7 +391,7 @@ CommunicationTools::buildNeighborPartitionBoundaryObjects( ObjectManagerBase & m
{
return std::cref( globalPartitionBoundaryObjectsIndices );
};
array1d< array1d< globalIndex > > const neighborPartitionBoundaryObjects = exchange( getCommID(), allNeighbors, data );
array1d< array1d< globalIndex > > const neighborPartitionBoundaryObjects = exchange( allNeighbors, data );

integer const numNeighbors = LvArray::integerConversion< integer >( allNeighbors.size() );
for( integer i = 0; i < numNeighbors; ++i )
Expand Down Expand Up @@ -566,7 +566,7 @@ void CommunicationTools::findMatchedPartitionBoundaryNodes( NodeManager & nodeMa
{
return req.at( allNeighbors[i].neighborRank() );
};
array1d< array1d< globalIndex > > const nodesRequestedByNeighbors = exchange( getCommID(), allNeighbors, data );
array1d< array1d< globalIndex > > const nodesRequestedByNeighbors = exchange( allNeighbors, data );

// Then we store the requested nodes for each receiver.
for( integer i = 0; i < numNeighbors; ++i )
Expand Down Expand Up @@ -804,7 +804,7 @@ void CommunicationTools::setupGhosts( MeshLevel & meshLevel,
bool const unorderedComms )
{
GEOS_MARK_FUNCTION;
MPI_iCommData commData( getCommID() );
MPI_iCommData commData;
commData.resize( neighbors.size() );

NodeManager & nodeManager = meshLevel.getNodeManager();
Expand Down Expand Up @@ -1092,7 +1092,7 @@ void CommunicationTools::synchronizeFields( FieldIdentifiers const & fieldsToBeS
std::vector< NeighborCommunicator > & neighbors,
bool onDevice )
{
MPI_iCommData icomm( getCommID() );
MPI_iCommData icomm;
icomm.resize( neighbors.size() );
synchronizePackSendRecvSizes( fieldsToBeSync, mesh, neighbors, icomm, onDevice );
synchronizePackSendRecv( fieldsToBeSync, mesh, neighbors, icomm, onDevice );
Expand Down
5 changes: 3 additions & 2 deletions src/coreComponents/mesh/mpiCommunications/MPI_iCommData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
*/

#include "MPI_iCommData.hpp"
#include "CommunicationTools.hpp"

namespace geos
{

MPI_iCommData::MPI_iCommData( int const inputCommID ):
MPI_iCommData::MPI_iCommData():
m_size( 0 ),
m_commID( inputCommID ), // CommunicationTools::getInstance().getCommID() ),
m_commID( CommunicationTools::getInstance().getCommID() ),
m_mpiSendBufferRequest(),
m_mpiRecvBufferRequest(),
m_mpiSendBufferStatus(),
Expand Down
4 changes: 2 additions & 2 deletions src/coreComponents/mesh/mpiCommunications/MPI_iCommData.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class MPI_iCommData
* @param inputCommID The CommID integer that indicates what communication
* pipeline to use for a set of neighbor communications.
*/
MPI_iCommData( int const inputCommID );
MPI_iCommData();

/// Default destructor
~MPI_iCommData();
Expand Down Expand Up @@ -96,7 +96,7 @@ class MPI_iCommData
int m_size;

/// The integer ID for the set of communication pipelines
int m_commID;
CommID m_commID;

/// A collection of field names keyed on object keys to pack/unpack from
/// communication pipeline.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
m_maxForce( 0.0 ),
m_maxNumResolves( 10 ),
m_strainTheory( 0 ),
m_iComm( CommunicationTools::getInstance().getCommID() ),
m_isFixedStressPoromechanicsUpdate( false )
{

Expand Down Expand Up @@ -571,6 +570,7 @@
solidMechanics::arrayView2dLayoutIncrDisplacement const & uhat = nodes.getField< solidMechanics::incrementalDisplacement >();
solidMechanics::arrayView2dLayoutAcceleration const & acc = nodes.getField< solidMechanics::acceleration >();

MPI_iCommData m_iComm;

Check warning on line 573 in src/coreComponents/physicsSolvers/solidMechanics/SolidMechanicsLagrangianFEM.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/solidMechanics/SolidMechanicsLagrangianFEM.cpp#L573

Added line #L573 was not covered by tests
FieldIdentifiers fieldsToBeSync;
fieldsToBeSync.addFields( FieldLocation::Node,
{ solidMechanics::velocity::key(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ class SolidMechanicsLagrangianFEM : public SolverBase
real64 m_maxForce = 0.0;
integer m_maxNumResolves;
integer m_strainTheory;
MPI_iCommData m_iComm;
// MPI_iCommData m_iComm;
bool m_isFixedStressPoromechanicsUpdate;

/// Rigid body modes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
SolverBase( name, parent ),
m_solverProfiling( 0 ),
m_timeIntegrationOption( TimeIntegrationOption::ExplicitDynamic ),
m_iComm( CommunicationTools::getInstance().getCommID() ),
// m_iComm( CommunicationTools::getInstance().getCommID() ),
m_prescribedBcTable( 0 ),
m_boundaryConditionTypes(),
m_bcTable(),
Expand Down Expand Up @@ -940,6 +940,7 @@
MeshLevel & mesh = grid.getBaseDiscretization();
NodeManager & nodeManager = mesh.getNodeManager();

MPI_iCommData m_iComm;

Check warning on line 943 in src/coreComponents/physicsSolvers/solidMechanics/SolidMechanicsMPM.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/solidMechanics/SolidMechanicsMPM.cpp#L943

Added line #L943 was not covered by tests
m_iComm.resize( domain.getNeighbors().size() );


Expand Down Expand Up @@ -1258,6 +1259,9 @@
fieldsToBeSynced.addFields( FieldLocation::Node, fieldNames );
std::vector< NeighborCommunicator > & neighbors = domain.getNeighbors();

MPI_iCommData m_iComm;
m_iComm.resize( neighbors.size() );

Check warning on line 1263 in src/coreComponents/physicsSolvers/solidMechanics/SolidMechanicsMPM.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/solidMechanics/SolidMechanicsMPM.cpp#L1262-L1263

Added lines #L1262 - L1263 were not covered by tests

// (2) Swap send and receive indices so we can sum from ghost to master
for( size_t n=0; n<neighbors.size(); n++ )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,6 @@ class SolidMechanicsMPM : public SolverBase
std::vector< std::string > m_profilingLabels;

TimeIntegrationOption m_timeIntegrationOption;
MPI_iCommData m_iComm;

int m_prescribedBcTable;
array1d< int > m_boundaryConditionTypes; // TODO: Surely there's a way to have just one variable here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@
//************************************************************************************************
// We need to send over the new embedded surfaces and related objects for those whose parents are ghosts on neighbors.

MPI_iCommData commData( CommunicationTools::getInstance().getCommID() );
MPI_iCommData commData;

Check warning on line 386 in src/coreComponents/physicsSolvers/surfaceGeneration/EmbeddedSurfacesParallelSynchronization.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/surfaceGeneration/EmbeddedSurfacesParallelSynchronization.cpp#L386

Added line #L386 was not covered by tests
commData.resize( neighbors.size());
for( unsigned int neighborIndex=0; neighborIndex<neighbors.size(); ++neighborIndex )
{
Expand Down Expand Up @@ -458,7 +458,7 @@
//************************************************************************************************
// We need to send over the new embedded surfaces and related objects for those whose parents are ghosts on neighbors.

MPI_iCommData commData( CommunicationTools::getInstance().getCommID() );
MPI_iCommData commData;

Check warning on line 461 in src/coreComponents/physicsSolvers/surfaceGeneration/EmbeddedSurfacesParallelSynchronization.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/surfaceGeneration/EmbeddedSurfacesParallelSynchronization.cpp#L461

Added line #L461 was not covered by tests
commData.resize( neighbors.size());
for( unsigned int neighborIndex=0; neighborIndex<neighbors.size(); ++neighborIndex )
{
Expand Down Expand Up @@ -528,8 +528,7 @@
std::vector< NeighborCommunicator > & neighbors,
string const fractureRegionName )
{
MPI_iCommData commDataJunk( CommunicationTools::getInstance().getCommID() );
MPI_iCommData commData( CommunicationTools::getInstance().getCommID() );
MPI_iCommData commData;

Check warning on line 531 in src/coreComponents/physicsSolvers/surfaceGeneration/EmbeddedSurfacesParallelSynchronization.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/surfaceGeneration/EmbeddedSurfacesParallelSynchronization.cpp#L531

Added line #L531 was not covered by tests
commData.resize( neighbors.size());
for( unsigned int neighborIndex=0; neighborIndex<neighbors.size(); ++neighborIndex )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@
// b) the modified objects to the owning ranks.

// pack the buffers, and send the size of the buffers
MPI_iCommData commData( CommunicationTools::getInstance().getCommID() );
MPI_iCommData commData;

Check warning on line 816 in src/coreComponents/physicsSolvers/surfaceGeneration/ParallelTopologyChange.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/surfaceGeneration/ParallelTopologyChange.cpp#L816

Added line #L816 was not covered by tests
commData.resize( neighbors.size() );
for( unsigned int neighborIndex=0; neighborIndex<neighbors.size(); ++neighborIndex )
{
Expand Down Expand Up @@ -905,7 +905,7 @@



MPI_iCommData commData2( CommunicationTools::getInstance().getCommID() );
MPI_iCommData commData2;

Check warning on line 908 in src/coreComponents/physicsSolvers/surfaceGeneration/ParallelTopologyChange.cpp

View check run for this annotation

Codecov / codecov/patch

src/coreComponents/physicsSolvers/surfaceGeneration/ParallelTopologyChange.cpp#L908

Added line #L908 was not covered by tests
commData2.resize( neighbors.size());
for( unsigned int neighborIndex=0; neighborIndex<neighbors.size(); ++neighborIndex )
{
Expand Down