From b7cbd59a13f36eae6fae7ce94fc41a5a2dd6c720 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Wed, 19 Oct 2022 13:02:07 -0500 Subject: [PATCH 1/4] Changing types to mitigate overflow problems --- src/volume_calc.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/volume_calc.cpp b/src/volume_calc.cpp index 3ae98fdc388..002171d4974 100644 --- a/src/volume_calc.cpp +++ b/src/volume_calc.cpp @@ -99,9 +99,9 @@ vector VolumeCalculation::execute() const { // Shared data that is collected from all threads int n = domain_ids_.size(); - vector> master_indices( + vector> master_indices( n); // List of material indices for each domain - vector> master_hits( + vector> master_hits( n); // Number of hits for each material in each domain int iterations = 0; @@ -281,7 +281,7 @@ vector VolumeCalculation::execute() const #endif if (mpi::master) { - int total_hits = 0; + size_t total_hits = 0; for (int j = 0; j < master_indices[i_domain].size(); ++j) { total_hits += master_hits[i_domain][j]; double f = From 2a802a11a4dd7b08b15c0851927f9439c6faf892 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Wed, 19 Oct 2022 13:23:26 -0500 Subject: [PATCH 2/4] Add warning of possible overflow. --- include/openmc/constants.h | 5 +++++ src/volume_calc.cpp | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/include/openmc/constants.h b/include/openmc/constants.h index fa2251b84dc..eadff1726f3 100644 --- a/include/openmc/constants.h +++ b/include/openmc/constants.h @@ -339,6 +339,11 @@ enum class RunMode { enum class GeometryType { CSG, DAG }; +//============================================================================== +// Volume Calculation Constants + +constexpr size_t SIZE_T_MAX {std::numeric_limits::max()}; + } // namespace openmc #endif // OPENMC_CONSTANTS_H diff --git a/src/volume_calc.cpp b/src/volume_calc.cpp index 002171d4974..56134f367af 100644 --- a/src/volume_calc.cpp +++ b/src/volume_calc.cpp @@ -225,6 +225,13 @@ vector VolumeCalculation::execute() const iterations++; size_t total_samples = iterations * n_samples_; + // warn user if total sample size is greater than what the size_t type can + // represent + if (total_samples > SIZE_T_MAX) { + warning("The number of samples has exceeded the size_t type. Volume " + "results may be inaccurate."); + } + // reset double trigger_val = -INFTY; From 06729507ce333ead873a78a3456d49bece14a4aa Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 20 Oct 2022 20:46:47 -0500 Subject: [PATCH 3/4] Moving to uint64_t and updating MPI types as requested by @paulromano --- include/openmc/constants.h | 3 ++- include/openmc/volume_calc.h | 7 +++++-- src/volume_calc.cpp | 38 +++++++++++++++++++----------------- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/include/openmc/constants.h b/include/openmc/constants.h index eadff1726f3..8bb2cb1bfa2 100644 --- a/include/openmc/constants.h +++ b/include/openmc/constants.h @@ -5,6 +5,7 @@ #define OPENMC_CONSTANTS_H #include +#include #include #include "openmc/array.h" @@ -342,7 +343,7 @@ enum class GeometryType { CSG, DAG }; //============================================================================== // Volume Calculation Constants -constexpr size_t SIZE_T_MAX {std::numeric_limits::max()}; +constexpr uint64_t UINT64_T_MAX {std::numeric_limits::max()}; } // namespace openmc diff --git a/include/openmc/volume_calc.h b/include/openmc/volume_calc.h index db96f250f96..2efd955f04b 100644 --- a/include/openmc/volume_calc.h +++ b/include/openmc/volume_calc.h @@ -1,6 +1,9 @@ #ifndef OPENMC_VOLUME_CALC_H #define OPENMC_VOLUME_CALC_H +#include +#include + #include "openmc/array.h" #include "openmc/position.h" #include "openmc/tallies/trigger.h" @@ -10,7 +13,6 @@ #include "xtensor/xtensor.hpp" #include -#include namespace openmc { @@ -69,7 +71,8 @@ class VolumeCalculation { //! \param[in] i_material Index in global materials vector //! \param[in,out] indices Vector of material indices //! \param[in,out] hits Number of hits corresponding to each material - void check_hit(int i_material, vector& indices, vector& hits) const; + void check_hit( + int i_material, vector& indices, vector& hits) const; }; //============================================================================== diff --git a/src/volume_calc.cpp b/src/volume_calc.cpp index 56134f367af..126edd9277f 100644 --- a/src/volume_calc.cpp +++ b/src/volume_calc.cpp @@ -99,16 +99,16 @@ vector VolumeCalculation::execute() const { // Shared data that is collected from all threads int n = domain_ids_.size(); - vector> master_indices( + vector> master_indices( n); // List of material indices for each domain - vector> master_hits( + vector> master_hits( n); // Number of hits for each material in each domain int iterations = 0; // Divide work over MPI processes - size_t min_samples = n_samples_ / mpi::n_procs; - size_t remainder = n_samples_ % mpi::n_procs; - size_t i_start, i_end; + uint64_t min_samples = n_samples_ / mpi::n_procs; + uint64_t remainder = n_samples_ % mpi::n_procs; + uint64_t i_start, i_end; if (mpi::rank < remainder) { i_start = (min_samples + 1) * mpi::rank; i_end = i_start + min_samples + 1; @@ -123,14 +123,14 @@ vector VolumeCalculation::execute() const #pragma omp parallel { // Variables that are private to each thread - vector> indices(n); - vector> hits(n); + vector> indices(n); + vector> hits(n); Particle p; // Sample locations and count hits #pragma omp for for (size_t i = i_start; i < i_end; i++) { - int64_t id = iterations * n_samples_ + i; + uint64_t id = iterations * n_samples_ + i; uint64_t seed = init_seed(id, STREAM_VOLUME); p.n_coord() = 1; @@ -223,12 +223,13 @@ vector VolumeCalculation::execute() const // bump iteration counter and get total number // of samples at this point iterations++; - size_t total_samples = iterations * n_samples_; + uint64_t total_samples = iterations * n_samples_; // warn user if total sample size is greater than what the size_t type can // represent - if (total_samples > SIZE_T_MAX) { - warning("The number of samples has exceeded the size_t type. Volume " + if (total_samples > UINT64_T_MAX) { + warning("The number of samples has exceeded the type used to track hits. " + "Volume " "results may be inaccurate."); } @@ -253,10 +254,11 @@ vector VolumeCalculation::execute() const if (mpi::master) { for (int j = 1; j < mpi::n_procs; j++) { int q; + // retrieve results MPI_Recv( - &q, 1, MPI_INTEGER, j, 2 * j, mpi::intracomm, MPI_STATUS_IGNORE); - vector buffer(2 * q); - MPI_Recv(buffer.data(), 2 * q, MPI_INTEGER, j, 2 * j + 1, + &q, 1, MPI_UINT64_T, j, 2 * j, mpi::intracomm, MPI_STATUS_IGNORE); + vector buffer(2 * q); + MPI_Recv(buffer.data(), 2 * q, MPI_UINT64_T, j, 2 * j + 1, mpi::intracomm, MPI_STATUS_IGNORE); for (int k = 0; k < q; ++k) { bool already_added = false; @@ -275,14 +277,14 @@ vector VolumeCalculation::execute() const } } else { int q = master_indices[i_domain].size(); - vector buffer(2 * q); + vector buffer(2 * q); for (int k = 0; k < q; ++k) { buffer[2 * k] = master_indices[i_domain][k]; buffer[2 * k + 1] = master_hits[i_domain][k]; } - MPI_Send(&q, 1, MPI_INTEGER, 0, 2 * mpi::rank, mpi::intracomm); - MPI_Send(buffer.data(), 2 * q, MPI_INTEGER, 0, 2 * mpi::rank + 1, + MPI_Send(&q, 1, MPI_UINT64_T, 0, 2 * mpi::rank, mpi::intracomm); + MPI_Send(buffer.data(), 2 * q, MPI_UINT64_T, 0, 2 * mpi::rank + 1, mpi::intracomm); } #endif @@ -471,7 +473,7 @@ void VolumeCalculation::to_hdf5( } void VolumeCalculation::check_hit( - int i_material, vector& indices, vector& hits) const + int i_material, vector& indices, vector& hits) const { // Check if this material was previously hit and if so, increment count From 93065d188bdffe09138b6ecc34c686ae4d52432d Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 20 Oct 2022 20:54:43 -0500 Subject: [PATCH 4/4] Fixing sign for uint max comparison --- src/volume_calc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/volume_calc.cpp b/src/volume_calc.cpp index 126edd9277f..e36b8890413 100644 --- a/src/volume_calc.cpp +++ b/src/volume_calc.cpp @@ -227,7 +227,7 @@ vector VolumeCalculation::execute() const // warn user if total sample size is greater than what the size_t type can // represent - if (total_samples > UINT64_T_MAX) { + if (total_samples == UINT64_T_MAX) { warning("The number of samples has exceeded the type used to track hits. " "Volume " "results may be inaccurate.");