From cd4fdf604d458ecde86390d47d4c4427eb591bc0 Mon Sep 17 00:00:00 2001
From: Cristian C Lalescu <Cristian.Lalescu@mpcdf.mpg.de>
Date: Fri, 5 Mar 2021 06:52:59 +0100
Subject: [PATCH] fixes buffer usage

array was allocated, but not cleaned up before use.
I now set the buffer to zero.
---
 cpp/particles/p2p/p2p_distr_mpi.hpp | 116 ++++++++++++++++++++--------
 1 file changed, 82 insertions(+), 34 deletions(-)

diff --git a/cpp/particles/p2p/p2p_distr_mpi.hpp b/cpp/particles/p2p/p2p_distr_mpi.hpp
index 30c6ec63..b8b16d9f 100644
--- a/cpp/particles/p2p/p2p_distr_mpi.hpp
+++ b/cpp/particles/p2p/p2p_distr_mpi.hpp
@@ -129,6 +129,8 @@ protected:
                         = dataBuffer[srcData*sizeElement + idxVal];
             }
         }
+
+        buffer->resize(0);
     }
 
     static int foundGridFactor(const real_number in_cutoff_radius, const std::array<real_number,3>& in_spatial_box_width){
@@ -342,9 +344,10 @@ public:
                                 current_offset_particles_for_partition[idxPartition],
                                 current_my_nb_particles_per_partition[idxPartition],
                                 part_to_sort.data(),
-                                       particles_positions,
-                                       &buffer);
-                for(int idx_rhs = 0 ; idx_rhs < nb_rhs ; ++idx_rhs){
+                                particles_positions,
+                                &buffer);
+                for(int idx_rhs = 0 ; idx_rhs < nb_rhs ; ++idx_rhs)
+                {
                     permute_copy<real_number, size_particle_rhs>(
                                     current_offset_particles_for_partition[idxPartition],
                                     current_my_nb_particles_per_partition[idxPartition],
@@ -356,14 +359,14 @@ public:
                                 current_offset_particles_for_partition[idxPartition],
                                 current_my_nb_particles_per_partition[idxPartition],
                                 part_to_sort.data(),
-                                       inout_index_particles,
-                                       &buffer);
+                                inout_index_particles,
+                                &buffer);
                 permute_copy<long int, 1>(
                                 current_offset_particles_for_partition[idxPartition],
                                 current_my_nb_particles_per_partition[idxPartition],
                                 part_to_sort.data(),
-                                       particles_coord.get(),
-                                       &buffer);
+                                particles_coord.get(),
+                                &buffer);
             }
         }
 
@@ -497,27 +500,40 @@ public:
                     whatNext.emplace_back(std::pair<Action,int>{NOTHING_TODO, -1});
                     mpiRequests.emplace_back();
                     assert(descriptor.nbParticlesToExchange*size_particle_positions < std::numeric_limits<int>::max());
-                    AssertMpi(MPI_Isend(const_cast<real_number*>(&particles_positions[particles_offset_layers[my_nb_cell_levels-descriptor.nbLevelsToExchange]*size_particle_positions]),
-                              int(descriptor.nbParticlesToExchange*size_particle_positions), particles_utils::GetMpiType(real_number()),
-                              descriptor.destProc, TAG_POSITION_PARTICLES,
-                              current_com, &mpiRequests.back()));
+                    AssertMpi(MPI_Isend(
+                                const_cast<real_number*>(&particles_positions[particles_offset_layers[my_nb_cell_levels-descriptor.nbLevelsToExchange]*size_particle_positions]),
+                                int(descriptor.nbParticlesToExchange*size_particle_positions),
+                                particles_utils::GetMpiType(real_number()),
+                                descriptor.destProc,
+                                TAG_POSITION_PARTICLES,
+                                current_com,
+                                &mpiRequests.back()));
 
                     whatNext.emplace_back(std::pair<Action,int>{NOTHING_TODO, -1});
                     mpiRequests.emplace_back();
                     assert(descriptor.nbParticlesToExchange*size_particle_positions < std::numeric_limits<int>::max());
-                    AssertMpi(MPI_Isend(const_cast<partsize_t*>(&inout_index_particles[particles_offset_layers[my_nb_cell_levels-descriptor.nbLevelsToExchange]]),
-                              int(descriptor.nbParticlesToExchange), particles_utils::GetMpiType(partsize_t()),
-                              descriptor.destProc, TAG_INDEX_PARTICLES,
-                              current_com, &mpiRequests.back()));
+                    AssertMpi(MPI_Isend(
+                                const_cast<partsize_t*>(&inout_index_particles[particles_offset_layers[my_nb_cell_levels-descriptor.nbLevelsToExchange]]),
+                                int(descriptor.nbParticlesToExchange),
+                                particles_utils::GetMpiType(partsize_t()),
+                                descriptor.destProc,
+                                TAG_INDEX_PARTICLES,
+                                current_com,
+                                &mpiRequests.back()));
 
                     assert(descriptor.toRecvAndMerge == nullptr);
                     descriptor.toRecvAndMerge.reset(new real_number[descriptor.nbParticlesToExchange*size_particle_rhs]);
                     whatNext.emplace_back(std::pair<Action,int>{MERGE_PARTICLES, idxDescr});
                     mpiRequests.emplace_back();
                     assert(descriptor.nbParticlesToExchange*size_particle_rhs < std::numeric_limits<int>::max());
-                    AssertMpi(MPI_Irecv(descriptor.toRecvAndMerge.get(), int(descriptor.nbParticlesToExchange*size_particle_rhs),
-                                        particles_utils::GetMpiType(real_number()), descriptor.destProc, TAG_RESULT_PARTICLES,
-                                        current_com, &mpiRequests.back()));
+                    AssertMpi(MPI_Irecv(
+                                descriptor.toRecvAndMerge.get(),
+                                int(descriptor.nbParticlesToExchange*size_particle_rhs),
+                                particles_utils::GetMpiType(real_number()),
+                                descriptor.destProc,
+                                TAG_RESULT_PARTICLES,
+                                current_com,
+                                &mpiRequests.back()));
                 }
             }
             else{
@@ -526,9 +542,14 @@ public:
 #endif
                 whatNext.emplace_back(std::pair<Action,int>{RECV_PARTICLES, idxDescr});
                 mpiRequests.emplace_back();
-                AssertMpi(MPI_Irecv(&descriptor.nbParticlesToExchange,
-                      1, particles_utils::GetMpiType(partsize_t()), descriptor.destProc, TAG_NB_PARTICLES,
-                      current_com, &mpiRequests.back()));
+                AssertMpi(MPI_Irecv(
+                            &descriptor.nbParticlesToExchange,
+                            1,
+                            particles_utils::GetMpiType(partsize_t()),
+                            descriptor.destProc,
+                            TAG_NB_PARTICLES,
+                            current_com,
+                            &mpiRequests.back()));
             }
         }
 
@@ -538,8 +559,14 @@ public:
                 std::vector<int> willsendall(nb_processes_involved*nb_processes_involved, 0);// TODO debug
                 std::vector<int> willrecvall(nb_processes_involved*nb_processes_involved, 0);// TODO debug
 
-                MPI_Gather(willrecv.data(), nb_processes_involved, MPI_INT, willrecvall.data(),
-                            nb_processes_involved, MPI_INT, 0, MPI_COMM_WORLD);
+                MPI_Gather(willrecv.data(),
+                           nb_processes_involved,
+                           MPI_INT,
+                           willrecvall.data(),
+                           nb_processes_involved,
+                           MPI_INT,
+                           0,
+                           MPI_COMM_WORLD);
                 MPI_Gather(willsend.data(), nb_processes_involved, MPI_INT, willsendall.data(),
                             nb_processes_involved, MPI_INT, 0, MPI_COMM_WORLD);
 
@@ -605,17 +632,27 @@ public:
                             whatNext.emplace_back(std::pair<Action,int>{COMPUTE_PARTICLES, releasedAction.second});
                             mpiRequests.emplace_back();
                             assert(NbParticlesToReceive*size_particle_positions < std::numeric_limits<int>::max());
-                            AssertMpi(MPI_Irecv(descriptor.toCompute.get(), int(NbParticlesToReceive*size_particle_positions),
-                                                particles_utils::GetMpiType(real_number()), destProc, TAG_POSITION_PARTICLES,
-                                                current_com, &mpiRequests.back()));
+                            AssertMpi(MPI_Irecv(
+                                        descriptor.toCompute.get(),
+                                        int(NbParticlesToReceive*size_particle_positions),
+                                        particles_utils::GetMpiType(real_number()),
+                                        destProc,
+                                        TAG_POSITION_PARTICLES,
+                                        current_com,
+                                        &mpiRequests.back()));
 
                             descriptor.indexes.reset(new partsize_t[NbParticlesToReceive]);
                             whatNext.emplace_back(std::pair<Action,int>{COMPUTE_PARTICLES, releasedAction.second});
                             mpiRequests.emplace_back();
                             assert(NbParticlesToReceive*size_particle_positions < std::numeric_limits<int>::max());
-                            AssertMpi(MPI_Irecv(descriptor.indexes.get(), int(NbParticlesToReceive),
-                                                particles_utils::GetMpiType(partsize_t()), destProc, TAG_INDEX_PARTICLES,
-                                                current_com, &mpiRequests.back()));
+                            AssertMpi(MPI_Irecv(
+                                        descriptor.indexes.get(),
+                                        int(NbParticlesToReceive),
+                                        particles_utils::GetMpiType(partsize_t()),
+                                        destProc,
+                                        TAG_INDEX_PARTICLES,
+                                        current_com,
+                                        &mpiRequests.back()));
                         }
                     }
 
@@ -635,7 +672,11 @@ public:
 
                             assert(descriptor.toCompute != nullptr);
                             assert(descriptor.indexes != nullptr);
+                            // allocate rhs buffer
                             descriptor.results.reset(new real_number[NbParticlesToReceive*size_particle_rhs]);
+                            // clean up rhs buffer
+                            std::fill_n(descriptor.results.get(), NbParticlesToReceive*size_particle_rhs, 0);
+                            // precompute rhs buffer (?)
                             computer_thread.template init_result_array<size_particle_rhs>(descriptor.results.get(), NbParticlesToReceive);
 
                             // Compute
@@ -718,9 +759,14 @@ public:
                             whatNext.emplace_back(std::pair<Action,int>{RELEASE_BUFFER_PARTICLES, releasedAction.second});
                             mpiRequests.emplace_back();
                             assert(NbParticlesToReceive*size_particle_rhs < std::numeric_limits<int>::max());
-                            AssertMpi(MPI_Isend(descriptor.results.get(), int(NbParticlesToReceive*size_particle_rhs),
-                                                particles_utils::GetMpiType(real_number()), destProc, TAG_RESULT_PARTICLES,
-                                                current_com, &mpiRequests.back()));
+                            AssertMpi(MPI_Isend(
+                                        descriptor.results.get(),
+                                        int(NbParticlesToReceive*size_particle_rhs),
+                                        particles_utils::GetMpiType(real_number()),
+                                        destProc,
+                                        TAG_RESULT_PARTICLES,
+                                        current_com,
+                                        &mpiRequests.back()));
                             delete[] descriptor.toCompute.release();
                             delete[] descriptor.indexes.release();
                         }
@@ -742,8 +788,10 @@ public:
                         NeighborDescriptor& descriptor = neigDescriptors[releasedAction.second];
                         assert(descriptor.isRecv == false);
                         assert(descriptor.toRecvAndMerge != nullptr);
-                        computer_thread.template reduce_particles_rhs<size_particle_rhs>(&particles_current_rhs[0][particles_offset_layers[my_nb_cell_levels-descriptor.nbLevelsToExchange]*size_particle_rhs],
-                                descriptor.toRecvAndMerge.get(), descriptor.nbParticlesToExchange);
+                        computer_thread.template reduce_particles_rhs<size_particle_rhs>(
+                                &particles_current_rhs[0][particles_offset_layers[my_nb_cell_levels-descriptor.nbLevelsToExchange]*size_particle_rhs],
+                                descriptor.toRecvAndMerge.get(),
+                                descriptor.nbParticlesToExchange);
                         delete[] descriptor.toRecvAndMerge.release();
                     }
                 }
-- 
GitLab