From db529a56728a8f7185e22d827b6b39cf59efb099 Mon Sep 17 00:00:00 2001 From: JOAN VINYALS YLLA CATALA Date: Thu, 29 Aug 2024 11:27:21 +0200 Subject: [PATCH 1/6] Update 3 files - /src/backends/dlb_talp_tree/dlb_talp_tree.cpp - /src/utils/parallelism_helper.cpp - /src/utils/parallelism_helper.hpp --- src/backends/dlb_talp_tree/dlb_talp_tree.cpp | 33 ++++++++++++++++++-- src/utils/parallelism_helper.cpp | 2 ++ src/utils/parallelism_helper.hpp | 2 ++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp index 19e7bcb..193e171 100644 --- a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp +++ b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp @@ -100,10 +100,39 @@ void DLBTalpTreeStrategy::Init() noexcept { void DLBTalpTreeStrategy::Finalize() noexcept { // Stop the top region which was automatically started by TALP. talp_profiling_strategy_.RegionStop({.name = top_region_}); + + std::site_t num_regions = regions_.size(); + std::vector region_hashes(num_regions); + for (auto ®ion : regions_) { + region_hashes.push_back(region.first); + } + + std::set unique_regions; + if (mpi_helper_.IsUsingMPI()) { + int mpi_size = mpi_helper_.getCommSize(); + std::vector dispalcements(mpi_size); + PMPI_Allgather(&num_regions, 1, MPI_INT, &dispalcements, mpi_size, MPI_INT, MPI_COMM_WORLD); + + std::size_t num_all_regions = std::accumulate(dispalcements.cbegin(), dispalcements.cend()); + + std::vector all_regions(num_all_regions); + PMPI_Allgatherv(®ion_hashes, num_regions, MPI_INT, &all_regions, num_all_regions, &dispalcements, MPI_INT, MPI_COMM_WORLD); + + + for (std::size_t region_hash : all_regions) { + unique_regions.insert(region_hash); + } + } else { + for (std::size_t region_hash : region_hashes) { + unique_regions.insert(region_hash); + } + } // Collect the POP metrics from all the regions - for (auto ®ion : regions_) { - std::string name = region_hash_to_string(region.first); + for (auto ®ion_hash : unique_regions) { + auto ®ion = region_[region_hash]; + + std::string name = region_hash_to_string(region_hash); if (region.second.name.compare(top_region_) == 0) { name = top_region_; } diff --git a/src/utils/parallelism_helper.cpp b/src/utils/parallelism_helper.cpp index 2955cc4..23434ea 100644 --- a/src/utils/parallelism_helper.cpp +++ b/src/utils/parallelism_helper.cpp @@ -52,6 +52,8 @@ bool MPIHelper::IsRankNumber(int asked_rank) const { return asked_rank == 0; } int MPIHelper::getRankNumber() const { return mpi_comm_rank; } +int MPIHelper::getCommSize() const { return mpi_comm_size; } + MPIHelper::MPIHelper() {} #endif diff --git a/src/utils/parallelism_helper.hpp b/src/utils/parallelism_helper.hpp index 7561d02..e92bf21 100644 --- a/src/utils/parallelism_helper.hpp +++ b/src/utils/parallelism_helper.hpp @@ -23,6 +23,8 @@ class MPIHelper { bool IsRankNumber(int asked_rank) const; int getRankNumber() const; + + int getCommSize() const; }; #endif // NESMIK_PARALLELISM_HELPER_HPP -- GitLab From 64bb4395356b1e4b5d62f1c817a7c97f1da9cb47 Mon Sep 17 00:00:00 2001 From: JOAN VINYALS YLLA CATALA Date: Thu, 29 Aug 2024 15:38:59 +0200 Subject: [PATCH 2/6] Add MPI guards and fix some MPI->PMPI --- src/backends/detection/detection.cpp | 2 +- src/backends/dlb_talp_tree/dlb_talp_tree.cpp | 19 ++++++++++++------- src/utils/parallelism_helper.cpp | 6 +++--- src/utils/terminator.cpp | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/backends/detection/detection.cpp b/src/backends/detection/detection.cpp index 3fb20fa..2be1c92 100644 --- a/src/backends/detection/detection.cpp +++ b/src/backends/detection/detection.cpp @@ -44,7 +44,7 @@ ThreadStore getThreadStoreOfCaller() { double getTimeStamp() { #ifdef WITH_MPI - return MPI_Wtime(); + return PMPI_Wtime(); #else using clock = std::chrono::system_clock; auto now = clock::now(); diff --git a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp index 193e171..98c28f3 100644 --- a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp +++ b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp @@ -100,7 +100,7 @@ void DLBTalpTreeStrategy::Init() noexcept { void DLBTalpTreeStrategy::Finalize() noexcept { // Stop the top region which was automatically started by TALP. talp_profiling_strategy_.RegionStop({.name = top_region_}); - + std::site_t num_regions = regions_.size(); std::vector region_hashes(num_regions); for (auto ®ion : regions_) { @@ -108,21 +108,26 @@ void DLBTalpTreeStrategy::Finalize() noexcept { } std::set unique_regions; +#ifdef WITH_MPI if (mpi_helper_.IsUsingMPI()) { int mpi_size = mpi_helper_.getCommSize(); - std::vector dispalcements(mpi_size); - PMPI_Allgather(&num_regions, 1, MPI_INT, &dispalcements, mpi_size, MPI_INT, MPI_COMM_WORLD); + std::vector displacements(mpi_size); + PMPI_Allgather(&num_regions, 1, MPI_INT, &displacements, mpi_size, MPI_INT, + MPI_COMM_WORLD); - std::size_t num_all_regions = std::accumulate(dispalcements.cbegin(), dispalcements.cend()); + std::size_t num_all_regions = + std::accumulate(displacements.cbegin(), displacements.cend()); std::vector all_regions(num_all_regions); - PMPI_Allgatherv(®ion_hashes, num_regions, MPI_INT, &all_regions, num_all_regions, &dispalcements, MPI_INT, MPI_COMM_WORLD); - + PMPI_Allgatherv(®ion_hashes, num_regions, MPI_INT, &all_regions, + num_all_regions, &displacements, MPI_INT, MPI_COMM_WORLD); for (std::size_t region_hash : all_regions) { unique_regions.insert(region_hash); } - } else { + } else +#endif + { for (std::size_t region_hash : region_hashes) { unique_regions.insert(region_hash); } diff --git a/src/utils/parallelism_helper.cpp b/src/utils/parallelism_helper.cpp index 23434ea..e75f9ec 100644 --- a/src/utils/parallelism_helper.cpp +++ b/src/utils/parallelism_helper.cpp @@ -27,11 +27,11 @@ int MPIHelper::getRankNumber() const { return mpi_comm_rank; } MPIHelper::MPIHelper() { if (!mpi_information.valid) { int is_initialized; - MPI_Initialized(&is_initialized); + PMPI_Initialized(&is_initialized); mpi_information.mpi_is_initialized = static_cast(is_initialized); if (mpi_information.mpi_is_initialized) { - MPI_Comm_size(MPI_COMM_WORLD, &mpi_information.comm_size); - MPI_Comm_rank(MPI_COMM_WORLD, &mpi_information.rank); + PMPI_Comm_size(MPI_COMM_WORLD, &mpi_information.comm_size); + PMPI_Comm_rank(MPI_COMM_WORLD, &mpi_information.rank); } mpi_information.valid = true; } diff --git a/src/utils/terminator.cpp b/src/utils/terminator.cpp index b9b3ca0..527198f 100644 --- a/src/utils/terminator.cpp +++ b/src/utils/terminator.cpp @@ -13,7 +13,7 @@ void Terminator::exit(int exitcode) { } #ifdef WITH_MPI if (mpi_helper.IsUsingMPI()) { - MPI_Abort(MPI_COMM_WORLD, exitcode); + PMPI_Abort(MPI_COMM_WORLD, exitcode); } else #endif { -- GitLab From 5ad15ff5bd67fae7843e88d40603356f6526ee3e Mon Sep 17 00:00:00 2001 From: JOAN VINYALS YLLA CATALA Date: Thu, 29 Aug 2024 17:21:14 +0200 Subject: [PATCH 3/6] Fix MPI test and PMIP_Allgatherv --- src/backends/dlb_talp_tree/dlb_talp_tree.cpp | 62 ++++++++++++++++---- src/utils/parallelism_helper.cpp | 2 + tests/CMakeLists.txt | 10 +++- tests/cpp/TestCppTalpTreeMPI_MPMD.cpp | 15 +++++ 4 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 tests/cpp/TestCppTalpTreeMPI_MPMD.cpp diff --git a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp index 98c28f3..8391967 100644 --- a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp +++ b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp @@ -1,9 +1,11 @@ #include "dlb_talp_tree.hpp" #include +#include #include #include #include +#include #include #include #include @@ -101,26 +103,58 @@ void DLBTalpTreeStrategy::Finalize() noexcept { // Stop the top region which was automatically started by TALP. talp_profiling_strategy_.RegionStop({.name = top_region_}); - std::site_t num_regions = regions_.size(); - std::vector region_hashes(num_regions); + int num_regions = regions_.size(); + + std::vector region_hashes; + region_hashes.reserve(num_regions); + for (auto ®ion : regions_) { region_hashes.push_back(region.first); + std::cout << mpi_helper_.getRankNumber() << ": " << region.first << std::endl; } std::set unique_regions; #ifdef WITH_MPI if (mpi_helper_.IsUsingMPI()) { int mpi_size = mpi_helper_.getCommSize(); - std::vector displacements(mpi_size); - PMPI_Allgather(&num_regions, 1, MPI_INT, &displacements, mpi_size, MPI_INT, + std::vector counts; + std::vector displs; + counts.resize(mpi_size); + displs.resize(mpi_size); + + MPI_Allgather(&num_regions, 1, MPI_INT, counts.data(), 1, MPI_INT, MPI_COMM_WORLD); - std::size_t num_all_regions = - std::accumulate(displacements.cbegin(), displacements.cend()); + int num_all_regions = 0; + for (int i = 0; i < counts.size(); i++) { + num_all_regions += counts[i]; + } + + int last = 0; + for (int i = 0; i < counts.size(); i++) { + counts[i] *= sizeof(std::size_t); + displs[i] = last; + last += counts[i]; + } + std::cout << std::endl; + + std::vector all_regions(num_all_regions); + PMPI_Allgatherv( + // What we send + region_hashes.data(), + region_hashes.size() * sizeof(std::size_t), + MPI_BYTE, + + // What we receive + all_regions.data(), + counts.data(), + displs.data(), + MPI_BYTE, + + // To whom + MPI_COMM_WORLD + ); - std::vector all_regions(num_all_regions); - PMPI_Allgatherv(®ion_hashes, num_regions, MPI_INT, &all_regions, - num_all_regions, &displacements, MPI_INT, MPI_COMM_WORLD); for (std::size_t region_hash : all_regions) { unique_regions.insert(region_hash); @@ -135,16 +169,20 @@ void DLBTalpTreeStrategy::Finalize() noexcept { // Collect the POP metrics from all the regions for (auto ®ion_hash : unique_regions) { - auto ®ion = region_[region_hash]; + auto ®ion = regions_[region_hash]; std::string name = region_hash_to_string(region_hash); - if (region.second.name.compare(top_region_) == 0) { + if (region.name.compare(top_region_) == 0) { name = top_region_; } + if (mpi_helper_.IsRankNumber(0)) { + std::cout << region_hash << " : " << region.name << std::endl; + } + auto handle = DLB_MonitoringRegionRegister(name.c_str()); int dlb_error = - DLB_TALP_CollectPOPMetrics(handle, ®ion.second.pop_metrics); + DLB_TALP_CollectPOPMetrics(handle, ®ion.pop_metrics); if (dlb_error != DLB_SUCCESS) { // Warn about the error trying to close region name with hash diff --git a/src/utils/parallelism_helper.cpp b/src/utils/parallelism_helper.cpp index e75f9ec..7b63ba3 100644 --- a/src/utils/parallelism_helper.cpp +++ b/src/utils/parallelism_helper.cpp @@ -24,6 +24,8 @@ bool MPIHelper::IsRankNumber(int asked_rank) const { int MPIHelper::getRankNumber() const { return mpi_comm_rank; } +int MPIHelper::getCommSize() const { return mpi_comm_size; } + MPIHelper::MPIHelper() { if (!mpi_information.valid) { int is_initialized; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3840534..9223038 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -43,7 +43,15 @@ add_test(NAME TestCppTalpTree PRIVATE ${NESMIK_PUBLIC_HEADERS}) add_test(NAME TestCppTalpTreeMPI - COMMAND mpirun -np 2 TestCppTalpTree) + COMMAND TestCppTalpTree) + + add_executable(TestCppTalpTreeMPI_MPMD cpp/TestCppTalpTreeMPI_MPMD.cpp) + target_link_libraries(TestCppTalpTreeMPI_MPMD nesmik::nesmik MPI::MPI_CXX) + target_include_directories(TestCppTalpTreeMPI_MPMD + PRIVATE ${NESMIK_PUBLIC_HEADERS}) + + add_test(NAME TestCppTalpTreeMPI_MPMD + COMMAND TestCppTalpTreeMPI_MPMD) endif() diff --git a/tests/cpp/TestCppTalpTreeMPI_MPMD.cpp b/tests/cpp/TestCppTalpTreeMPI_MPMD.cpp new file mode 100644 index 0000000..600f2d3 --- /dev/null +++ b/tests/cpp/TestCppTalpTreeMPI_MPMD.cpp @@ -0,0 +1,15 @@ +#include + +#include "nesmik/nesmik.hpp" +int main() { + MPI_Init(NULL, NULL); + int rank; + MPI_Comm_rank(MPI_COMM_WORLD, &rank); + nesmik::nesmik_init(); + nesmik::region_start("Top"); + nesmik::region_start(std::to_string(rank)); + nesmik::region_stop(std::to_string(rank)); + nesmik::region_stop("Top"); + nesmik::nesmik_finalize(); + MPI_Finalize(); +} -- GitLab From 2263aa54d9df2e531595b4babbd21d2cae428612 Mon Sep 17 00:00:00 2001 From: JOAN VINYALS YLLA CATALA Date: Thu, 29 Aug 2024 17:26:50 +0200 Subject: [PATCH 4/6] Clang-fromat and remove std::cout --- src/backends/dlb_talp_tree/dlb_talp_tree.cpp | 43 +++++++------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp index 8391967..3dedbee 100644 --- a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp +++ b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp @@ -1,12 +1,12 @@ #include "dlb_talp_tree.hpp" #include -#include #include #include #include -#include #include +#include +#include #include #include @@ -110,7 +110,6 @@ void DLBTalpTreeStrategy::Finalize() noexcept { for (auto ®ion : regions_) { region_hashes.push_back(region.first); - std::cout << mpi_helper_.getRankNumber() << ": " << region.first << std::endl; } std::set unique_regions; @@ -123,38 +122,31 @@ void DLBTalpTreeStrategy::Finalize() noexcept { displs.resize(mpi_size); MPI_Allgather(&num_regions, 1, MPI_INT, counts.data(), 1, MPI_INT, - MPI_COMM_WORLD); + MPI_COMM_WORLD); int num_all_regions = 0; for (int i = 0; i < counts.size(); i++) { - num_all_regions += counts[i]; + num_all_regions += counts[i]; } int last = 0; for (int i = 0; i < counts.size(); i++) { - counts[i] *= sizeof(std::size_t); - displs[i] = last; - last += counts[i]; + counts[i] *= sizeof(std::size_t); + displs[i] = last; + last += counts[i]; } - std::cout << std::endl; std::vector all_regions(num_all_regions); PMPI_Allgatherv( - // What we send - region_hashes.data(), - region_hashes.size() * sizeof(std::size_t), - MPI_BYTE, + // What we send + region_hashes.data(), region_hashes.size() * sizeof(std::size_t), + MPI_BYTE, - // What we receive - all_regions.data(), - counts.data(), - displs.data(), - MPI_BYTE, - - // To whom - MPI_COMM_WORLD - ); + // What we receive + all_regions.data(), counts.data(), displs.data(), MPI_BYTE, + // To whom + MPI_COMM_WORLD); for (std::size_t region_hash : all_regions) { unique_regions.insert(region_hash); @@ -176,13 +168,8 @@ void DLBTalpTreeStrategy::Finalize() noexcept { name = top_region_; } - if (mpi_helper_.IsRankNumber(0)) { - std::cout << region_hash << " : " << region.name << std::endl; - } - auto handle = DLB_MonitoringRegionRegister(name.c_str()); - int dlb_error = - DLB_TALP_CollectPOPMetrics(handle, ®ion.pop_metrics); + int dlb_error = DLB_TALP_CollectPOPMetrics(handle, ®ion.pop_metrics); if (dlb_error != DLB_SUCCESS) { // Warn about the error trying to close region name with hash -- GitLab From 4fe279a2c370e73bf4d9ef1bd8ad332ac260db43 Mon Sep 17 00:00:00 2001 From: JOAN VINYALS YLLA CATALA Date: Thu, 29 Aug 2024 17:32:27 +0200 Subject: [PATCH 5/6] Apply 1 suggestion(s) to 1 file(s) --- src/backends/dlb_talp_tree/dlb_talp_tree.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp index 3dedbee..8f9d5b0 100644 --- a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp +++ b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp @@ -113,8 +113,8 @@ void DLBTalpTreeStrategy::Finalize() noexcept { } std::set unique_regions; -#ifdef WITH_MPI if (mpi_helper_.IsUsingMPI()) { +#ifdef WITH_MPI int mpi_size = mpi_helper_.getCommSize(); std::vector counts; std::vector displs; @@ -151,8 +151,8 @@ void DLBTalpTreeStrategy::Finalize() noexcept { for (std::size_t region_hash : all_regions) { unique_regions.insert(region_hash); } - } else #endif + } else { for (std::size_t region_hash : region_hashes) { unique_regions.insert(region_hash); -- GitLab From eb15a2f5debd5ee13ea972654dac9e78372a1be9 Mon Sep 17 00:00:00 2001 From: JOAN VINYALS YLLA CATALA Date: Thu, 29 Aug 2024 17:38:51 +0200 Subject: [PATCH 6/6] Clang-format --- src/backends/dlb_talp_tree/dlb_talp_tree.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp index 8f9d5b0..50802a7 100644 --- a/src/backends/dlb_talp_tree/dlb_talp_tree.cpp +++ b/src/backends/dlb_talp_tree/dlb_talp_tree.cpp @@ -152,8 +152,7 @@ void DLBTalpTreeStrategy::Finalize() noexcept { unique_regions.insert(region_hash); } #endif - } else - { + } else { for (std::size_t region_hash : region_hashes) { unique_regions.insert(region_hash); } -- GitLab