From e5be5c21103bbd2e710b52b75512eb6991869fcf Mon Sep 17 00:00:00 2001 From: Mark Olesen <Mark.Olesen@esi-group.com> Date: Fri, 25 Apr 2025 15:31:29 +0200 Subject: [PATCH] ENH: allow disabling of initial MPI_Comm_dup(MPI_COMM_WORLD,...) - can use -mpi-no-comm-dup to suppress the initial communicator duplication (to avoid potential deadlock with coupled processes). This is partly related to comments in merge-request !735 ENH: simplify parsing/removal of local -world option - can extract the world name in a single pass and also makes the parsing robuster. --- bin/tools/foamCreateCompletionCache | 3 +- bin/tools/help-filter | 3 +- src/OpenFOAM/db/IOstreams/Pstreams/UPstream.C | 2 + src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H | 3 + src/OpenFOAM/global/argList/argList.C | 10 ++- src/Pstream/mpi/UPstream.C | 78 +++++++++++++------ 6 files changed, 73 insertions(+), 26 deletions(-) diff --git a/bin/tools/foamCreateCompletionCache b/bin/tools/foamCreateCompletionCache index 7cc535afd9a..51d67d159fd 100755 --- a/bin/tools/foamCreateCompletionCache +++ b/bin/tools/foamCreateCompletionCache @@ -6,7 +6,7 @@ # \\ / A nd | www.openfoam.com # \\/ M anipulation | #------------------------------------------------------------------------------ -# Copyright (C) 2017-2023 OpenCFD Ltd. +# Copyright (C) 2017-2025 OpenCFD Ltd. #------------------------------------------------------------------------------ # License # This file is part of OpenFOAM, distributed under GPL-3.0-or-later. @@ -168,6 +168,7 @@ extractOptions() -e '/^-doc-source/d; /^-help-man/d;' \ -e '/^-hostRoots /d; /^-roots /d;' \ -e '/^-lib /d; /^-no-libs /d;' \ + -e '/^-mpi-.*/d;' \ -e '/^-[a-z]*-switch /d;' \ -e 'y/,/ /; s/=.*$/=/;' \ -e '/^-[^ ]* </{ s/^\(-[^ ]* <\).*$/\1/; p; d }' \ diff --git a/bin/tools/help-filter b/bin/tools/help-filter index df91b04d15d..fb4c492a20a 100755 --- a/bin/tools/help-filter +++ b/bin/tools/help-filter @@ -6,7 +6,7 @@ # \\ / A nd | www.openfoam.com # \\/ M anipulation | #------------------------------------------------------------------------------- -# Copyright (C) 2020-2023 OpenCFD Ltd. +# Copyright (C) 2020-2025 OpenCFD Ltd. #------------------------------------------------------------------------------ # License # This file is part of OpenFOAM, distributed under GPL-3.0-or-later. @@ -28,6 +28,7 @@ sed -ne '1,/^[Oo]ptions:/d' \ -e '/^-doc-source/d; /^-help-man/d;' \ -e '/^-hostRoots /d; /^-roots /d;' \ -e '/^-lib /d; /^-no-libs /d;' \ + -e '/^-mpi-.*/d;' \ -e '/^-[a-z]*-switch /d;' \ -e 'y/,/ /; s/=.*$/=/;' \ -e '/^-[^ ]* </{ s/^\(-[^ ]* <\).*$/\1/; p; d }' \ diff --git a/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.C b/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.C index 8bd92833829..934a61b3c39 100644 --- a/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.C +++ b/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.C @@ -857,6 +857,8 @@ bool Foam::UPstream::parRun_(false); bool Foam::UPstream::haveThreads_(false); +bool Foam::UPstream::noInitialCommDup_(false); + int Foam::UPstream::msgType_(1); diff --git a/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H b/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H index 76ff4f975e3..9c3b563d87d 100644 --- a/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H +++ b/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H @@ -397,6 +397,9 @@ private: //- Have support for threads? static bool haveThreads_; + //- Initial MPI_Comm_dup(MPI_COMM_WORLD,...) disabled? (default: false) + static bool noInitialCommDup_; + //- Standard transfer message type static int msgType_; diff --git a/src/OpenFOAM/global/argList/argList.C b/src/OpenFOAM/global/argList/argList.C index d04c26860aa..d39d1823f7b 100644 --- a/src/OpenFOAM/global/argList/argList.C +++ b/src/OpenFOAM/global/argList/argList.C @@ -128,7 +128,14 @@ Foam::argList::initValidTables::initValidTables() ( "mpi-threads", "Request use of MPI threads", - true // advanced option + true // advanced option + ); + + argList::addBoolOption + ( + "mpi-no-comm-dup", + "Disable initial MPI_Comm_dup()", + true // advanced option ); argList::addOption @@ -596,6 +603,7 @@ void Foam::argList::noParallel() removeOption("hostRoots"); removeOption("world"); removeOption("mpi-threads"); + removeOption("mpi-no-comm-dup"); validParOptions.clear(); } diff --git a/src/Pstream/mpi/UPstream.C b/src/Pstream/mpi/UPstream.C index 1b051b072fb..d7ea2ec0022 100644 --- a/src/Pstream/mpi/UPstream.C +++ b/src/Pstream/mpi/UPstream.C @@ -262,35 +262,62 @@ bool Foam::UPstream::init(int& argc, char**& argv, const bool needsThread) } - // Check argument list for local world - label worldIndex = -1; + // Check argument list for any of the following: + // - local world + // -> Extract world name and filter out '-world <name>' from argv list + // - mpi-no-comm-dup option + // -> disable initial comm_dup and filter out the option + + // Default handling of initial MPI_Comm_dup(MPI_COMM_WORLD,...) + UPstream::noInitialCommDup_ = false; + + // Local world name + word worldName; + for (int argi = 1; argi < argc; ++argi) { - if (strcmp(argv[argi], "-world") == 0) + const char *optName = argv[argi]; + if (optName[0] != '-') + { + continue; + } + ++optName; // Looks like an option, skip leading '-' + + if (strcmp(optName, "world") == 0) { - worldIndex = argi; if (argi+1 >= argc) { FatalErrorInFunction << "Missing world name for option '-world'" << nl << Foam::abort(FatalError); } - break; - } - } + worldName = argv[argi+1]; - // Extract world name and filter out '-world <name>' from argv list - word worldName; - if (worldIndex != -1) - { - worldName = argv[worldIndex+1]; - for (label i = worldIndex+2; i < argc; i++) + // Remove two arguments (-world name) + for (int i = argi+2; i < argc; ++i) + { + argv[i-2] = argv[i]; + } + argc -= 2; + --argi; // re-examine + } + else if (strcmp(optName, "mpi-no-comm-dup") == 0) { - argv[i-2] = argv[i]; + UPstream::noInitialCommDup_ = true; + + // Remove one argument + for (int i = argi+1; i < argc; ++i) + { + argv[i-1] = argv[i]; + } + --argc; + --argi; // re-examine } - argc -= 2; } + const bool hasLocalWorld(!worldName.empty()); + + int numProcs = 0, globalRanki = 0; MPI_Comm_rank(MPI_COMM_WORLD, &globalRanki); MPI_Comm_size(MPI_COMM_WORLD, &numProcs); @@ -314,7 +341,7 @@ bool Foam::UPstream::init(int& argc, char**& argv, const bool needsThread) << " world:" << worldName << endl; } - if (worldIndex == -1 && numProcs <= 1) + if (numProcs <= 1 && !(hasLocalWorld)) { FatalErrorInFunction << "attempt to run parallel on 1 processor" @@ -324,7 +351,7 @@ bool Foam::UPstream::init(int& argc, char**& argv, const bool needsThread) // Initialise parallel structure setParRun(numProcs, provided_thread_support == MPI_THREAD_MULTIPLE); - if (worldIndex != -1) + if (hasLocalWorld) { // Using local worlds. // During startup, so commWorld() == commGlobal() @@ -333,7 +360,7 @@ bool Foam::UPstream::init(int& argc, char**& argv, const bool needsThread) // Gather the names of all worlds and determine unique names/indices. // - // Minimize communication and use low-level MPI to relying on any + // Minimize communication and use low-level MPI to avoid relying on any // OpenFOAM structures which not yet have been created { @@ -665,11 +692,16 @@ void Foam::UPstream::allocateCommunicatorComponents } auto& mpiNewComm = PstreamGlobals::MPICommunicators_[index]; - // PstreamGlobals::pendingMPIFree_[index] = false; - // PstreamGlobals::MPICommunicators_[index] = MPI_COMM_WORLD; - - PstreamGlobals::pendingMPIFree_[index] = true; - MPI_Comm_dup(MPI_COMM_WORLD, &mpiNewComm); + if (UPstream::noInitialCommDup_) + { + PstreamGlobals::pendingMPIFree_[index] = false; + PstreamGlobals::MPICommunicators_[index] = MPI_COMM_WORLD; + } + else + { + PstreamGlobals::pendingMPIFree_[index] = true; + MPI_Comm_dup(MPI_COMM_WORLD, &mpiNewComm); + } MPI_Comm_rank(mpiNewComm, &myProcNo_[index]); -- GitLab