Make gmx_node_num/rank safer

With real MPI, MPI_Init() is called as the first thing, but with
thread-MPI, MPI is "initialized" only much later, when the actual
threads are started.  But code between these two points may expect to be
able to check whether the current rank is the master and/or whether
there are other ranks, and try to use these functions for that.

Previously, for thread-MPI builds, they accessed invalid memory and
returned essentially random values if called between these two init
points.  Now they behave as if the single thread that is executing at
that time is the single master rank.

Also document the behavior in more detail.

Change-Id: Ia3bef9c5899c528e315fe1310bde970dfec68769

diff --git a/src/gromacs/commandline/cmdlinemodulemanager.cpp b/src/gromacs/commandline/cmdlinemodulemanager.cpp
index 826d96fd9d9c24d86f7cc12764588dc3ccaef022..8ceab3e6a305e4121df80aed81f7de71ec904656 100644 (file)
--- a/src/gromacs/commandline/cmdlinemodulemanager.cpp
+++ b/src/gromacs/commandline/cmdlinemodulemanager.cpp
@@ -515,9 +515,7 @@ void CommandLineModuleManager::addHelpTopic(HelpTopicPointer topic)
 int CommandLineModuleManager::run(int argc, char *argv[])
 {
     CommandLineModuleInterface    *module;
-    // TODO: With thread-MPI, gmx_node_rank() returns random stuff here, so we
-    // need the first check.
-    const bool                     bMaster = (!gmx_mpi_initialized() || gmx_node_rank() == 0);
+    const bool                     bMaster = (gmx_node_rank() == 0);
     bool                           bQuiet  = impl_->bQuiet_ || !bMaster;
     CommandLineCommonOptionsHolder optionsHolder;
     try

diff --git a/src/gromacs/commandline/pargs.cpp b/src/gromacs/commandline/pargs.cpp
index 0f68e669fdbb644d3122ecbd3e0426573ea501d8..394205f590e047719c6e560e88adfbb02b8fffcc 100644 (file)
--- a/src/gromacs/commandline/pargs.cpp
+++ b/src/gromacs/commandline/pargs.cpp
@@ -567,9 +567,7 @@ gmx_bool parse_common_args(int *argc, char *argv[], unsigned long Flags,
             gmx::GlobalCommandLineHelpContext::get();
         if (context != NULL)
         {
-            // TODO: The first check should not be necessary, but with
-            // thread-MPI it is...
-            GMX_RELEASE_ASSERT(!gmx_mpi_initialized() || gmx_node_rank() == 0,
+            GMX_RELEASE_ASSERT(gmx_node_rank() == 0,
                                "Help output should be handled higher up and "
                                "only get called only on the master rank");
             gmx::CommandLineHelpWriter(options)

diff --git a/src/gromacs/utility/basenetwork.cpp b/src/gromacs/utility/basenetwork.cpp
index adc27e7c8ddf38ea5a6d004fcfe53bc9d9522f66..d96dd292b064c3eb40bcf6cd6d5f60c92f566bd4 100644 (file)
--- a/src/gromacs/utility/basenetwork.cpp
+++ b/src/gromacs/utility/basenetwork.cpp
@@ -91,6 +91,12 @@ int gmx_node_num(void)
 #ifndef GMX_MPI
     return 1;
 #else
+#ifdef GMX_THREAD_MPI
+    if (!gmx_mpi_initialized())
+    {
+        return 1;
+    }
+#endif
     int i;
     (void) MPI_Comm_size(MPI_COMM_WORLD, &i);
     return i;
@@ -102,6 +108,12 @@ int gmx_node_rank(void)
 #ifndef GMX_MPI
     return 0;
 #else
+#ifdef GMX_THREAD_MPI
+    if (!gmx_mpi_initialized())
+    {
+        return 0;
+    }
+#endif
     int i;
     (void) MPI_Comm_rank(MPI_COMM_WORLD, &i);
     return i;

diff --git a/src/gromacs/utility/basenetwork.h b/src/gromacs/utility/basenetwork.h
index 604f90a3a9d447494c696ab21b20a9261de89783..d93714c992b87fd3cd97e1a591398b7af796d62e 100644 (file)
--- a/src/gromacs/utility/basenetwork.h
+++ b/src/gromacs/utility/basenetwork.h
@@ -69,14 +69,34 @@ int gmx_gethostname(char *name, size_t len);
  *
  * The return value is `FALSE` if MPI_Init() has not been called, or if
  * \Gromacs has been compiled without MPI support.
- * For thread-MPI, always returns `TRUE`.
+ * For thread-MPI, returns `TRUE` when the threads have been started.
+ *
+ * Note that there is a lot of code in between MPI_Init() and the thread-MPI
+ * thread start where the return value is different depending on compilation
+ * options.
  */
 gmx_bool gmx_mpi_initialized(void);
 
-/** Returns the number of nodes. */
+/*! \brief
+ * Returns the number of nodes.
+ *
+ * For thread-MPI, returns one before the threads have been started.
+ * This allows code between the real MPI_Init() and the thread-MPI "init" to
+ * still use this function to check for serial/parallel status and work as
+ * expected: for thread-MPI, at that point they should behave as if the run was
+ * serial.
+ */
 int gmx_node_num(void);
 
-/** Returns the rank of the node. */
+/*! \brief
+ * Returns the rank of the node.
+ *
+ * For thread-MPI, returns zero before the threads have been started.
+ * This allows code between the real MPI_Init() and the thread-MPI "init" to
+ * still use this function to check for master node work as expected:
+ * for thread-MPI, at that point the only thread of execution should behave as
+ * if it the master node.
+ */
 int gmx_node_rank(void);
 
 /*! \brief