From 14818028f091e783c3500c774f7d3fe56753b5f1 Mon Sep 17 00:00:00 2001 From: Vegard Kippe Date: Fri, 28 Jun 2024 11:25:20 +0200 Subject: [PATCH 1/3] Handle empty or invalied OMP_NUM_THREADS by Flow default and warn if a valid value overrides --threads-per-process --- opm/simulators/flow/FlowMain.hpp | 33 +++++++++++++++++++------------- opm/simulators/flow/Main.hpp | 25 +++++++++++++----------- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/opm/simulators/flow/FlowMain.hpp b/opm/simulators/flow/FlowMain.hpp index 5480759ca..c5666664f 100644 --- a/opm/simulators/flow/FlowMain.hpp +++ b/opm/simulators/flow/FlowMain.hpp @@ -383,20 +383,27 @@ namespace Opm { mpi_size_ = comm.size(); #if _OPENMP - // if openMP is available, default to 2 threads per process unless - // OMP_NUM_THREADS is set or command line --threads-per-process used - if (!getenv("OMP_NUM_THREADS")) - { - int threads = 2; - const int requested_threads = Parameters::get(); - if (requested_threads > 0) - threads = requested_threads; - - // We are not limiting this to the number of processes - // reported by OpenMP as on some hardware (and some OpenMPI - // versions) this will be 1 when run with mpirun - omp_set_num_threads(threads); + // If openMP is available, default to 2 threads per process unless + // OMP_NUM_THREADS is set or command line --threads-per-process used. + // Issue a warning if both OMP_NUM_THREADS and --threads-per-process are set, + // but let the environment variable take precedence. + const int default_threads = 2; + const int requested_threads = Parameters::get(); + const char* env_var = getenv("OMP_NUM_THREADS"); + int omp_num_threads = -1; + try { + omp_num_threads = std::stoi(env_var ? env_var : ""); + // Warning in 'Main.hpp', where this code is duplicated + // if (requested_threads > 0) { + // OpmLog::warning("Environment variable OMP_NUM_THREADS takes precedence over the --threads-per-process cmdline argument."); + // } + } catch (const std::invalid_argument& e) { + omp_num_threads = requested_threads > 0 ? requested_threads : default_threads; } + // We are not limiting this to the number of processes + // reported by OpenMP as on some hardware (and some OpenMPI + // versions) this will be 1 when run with mpirun + omp_set_num_threads(omp_num_threads); #endif using ThreadManager = GetPropType; diff --git a/opm/simulators/flow/Main.hpp b/opm/simulators/flow/Main.hpp index cf6ff0814..470d4344b 100644 --- a/opm/simulators/flow/Main.hpp +++ b/opm/simulators/flow/Main.hpp @@ -713,21 +713,24 @@ private: // This function is called before the parallel OpenMP stuff gets initialized. // That initialization happends after the deck is read and we want this message. // Hence we duplicate the code of setupParallelism to get the number of threads. - if (std::getenv("OMP_NUM_THREADS")) { - threads = omp_get_max_threads(); - } - else { - threads = 2; - - const int input_threads = Parameters::get(); - - if (input_threads > 0) - threads = input_threads; + static bool first_time = true; + const int default_threads = 2; + const int requested_threads = Parameters::get(); + const char* env_var = getenv("OMP_NUM_THREADS"); + int omp_num_threads = -1; + try { + omp_num_threads = std::stoi(env_var ? env_var : ""); + if (first_time && requested_threads > 0 && FlowGenericVanguard::comm().rank()==0) { + std::cout << "Warning: Environment variable OMP_NUM_THREADS takes precedence over the --threads-per-process cmdline argument." << std::endl; + } + } catch (const std::invalid_argument& e) { + omp_num_threads = requested_threads > 0 ? requested_threads : default_threads; } + threads = omp_num_threads; + first_time = false; #else threads = 1; #endif - return threads; } From dd0895296707ef5a0905a600621978020536f079 Mon Sep 17 00:00:00 2001 From: Vegard Kippe Date: Thu, 11 Jul 2024 15:55:34 +0200 Subject: [PATCH 2/3] Update after move of ThreadsPerProcess from Properties to Parameters --- opm/simulators/flow/FlowMain.hpp | 2 +- opm/simulators/flow/Main.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opm/simulators/flow/FlowMain.hpp b/opm/simulators/flow/FlowMain.hpp index c5666664f..3ae346498 100644 --- a/opm/simulators/flow/FlowMain.hpp +++ b/opm/simulators/flow/FlowMain.hpp @@ -388,7 +388,7 @@ namespace Opm { // Issue a warning if both OMP_NUM_THREADS and --threads-per-process are set, // but let the environment variable take precedence. const int default_threads = 2; - const int requested_threads = Parameters::get(); + const int requested_threads = Parameters::get(); const char* env_var = getenv("OMP_NUM_THREADS"); int omp_num_threads = -1; try { diff --git a/opm/simulators/flow/Main.hpp b/opm/simulators/flow/Main.hpp index 470d4344b..5c4d913f5 100644 --- a/opm/simulators/flow/Main.hpp +++ b/opm/simulators/flow/Main.hpp @@ -20,7 +20,7 @@ You should have received a copy of the GNU General Public License along with OPM. If not, see . */ -#ifndef OPM_MAIN_HEADER_INCLUDED +#ifndef OPM_MAIN_HEADER_INCLUDEDp #define OPM_MAIN_HEADER_INCLUDED #include @@ -715,7 +715,7 @@ private: // Hence we duplicate the code of setupParallelism to get the number of threads. static bool first_time = true; const int default_threads = 2; - const int requested_threads = Parameters::get(); + const int requested_threads = Parameters::get(); const char* env_var = getenv("OMP_NUM_THREADS"); int omp_num_threads = -1; try { From a078eaacd3a7eb4ba53e576f5134a5a8cc39880a Mon Sep 17 00:00:00 2001 From: Vegard Kippe Date: Thu, 11 Jul 2024 18:15:16 +0200 Subject: [PATCH 3/3] Removing trailing 'p' --- opm/simulators/flow/Main.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opm/simulators/flow/Main.hpp b/opm/simulators/flow/Main.hpp index 5c4d913f5..45ba34c31 100644 --- a/opm/simulators/flow/Main.hpp +++ b/opm/simulators/flow/Main.hpp @@ -20,7 +20,7 @@ You should have received a copy of the GNU General Public License along with OPM. If not, see . */ -#ifndef OPM_MAIN_HEADER_INCLUDEDp +#ifndef OPM_MAIN_HEADER_INCLUDED #define OPM_MAIN_HEADER_INCLUDED #include