Clarifications for ResourceAssignment interface.

Add and update some documentation. Improve error messages. Improve
file naming.

Refs #3644

diff --git a/api/gmxapi/CMakeLists.txt b/api/gmxapi/CMakeLists.txt
index be182daa33355f101b9dcd2663b32156c842c798..fa145fceef4e821874c60e2ba294e24ab4198557 100644 (file)
--- a/api/gmxapi/CMakeLists.txt
+++ b/api/gmxapi/CMakeLists.txt
@@ -67,7 +67,7 @@ set_target_properties(gmxapi PROPERTIES
 # The include directory should be mostly empty so that we can use it internally as
 # the public interface include directory during build and testing.
 configure_file(include/gmxapiversion.h.in include/gmxapi/version.h)
-configure_file(include/multiprocessingresources.cmakein.h include/gmxapi/mpi/multiprocessingresources.h)
+configure_file(include/resourceassignment.cmakein.h include/gmxapi/mpi/resourceassignment.h)
 target_include_directories(gmxapi PUBLIC
                            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
                            $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>
@@ -90,6 +90,6 @@ install(DIRECTORY include/gmxapi
 # Install "configured" files from the build tree.
 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/include/gmxapi/version.h
         DESTINATION include/gmxapi)
-install(FILES ${CMAKE_CURRENT_BINARY_DIR}/include/gmxapi/mpi/multiprocessingresources.h
+install(FILES ${CMAKE_CURRENT_BINARY_DIR}/include/gmxapi/mpi/resourceassignment.h
         DESTINATION include/gmxapi/mpi)
 

diff --git a/api/gmxapi/cpp/CMakeLists.txt b/api/gmxapi/cpp/CMakeLists.txt
index f9d5a983738b291cb638f7b3715a68afd95fca38..8c6fc2992d19d703ec1eb87848d9fbba589ca0ec 100644 (file)
--- a/api/gmxapi/cpp/CMakeLists.txt
+++ b/api/gmxapi/cpp/CMakeLists.txt
@@ -35,7 +35,7 @@
 # This list file provides the Gromacs::gmxapi cmake target.
 
 target_sources(gmxapi PRIVATE
-               multiprocessingresources.cpp
+               resourceassignment.cpp
                context.cpp
                exceptions.cpp
                gmxapi.cpp

diff --git a/api/gmxapi/cpp/context.cpp b/api/gmxapi/cpp/context.cpp
index 9769d03b84a78fc72c7453a97999c6ad663ce747..903f3ce56d6ba6fcc24eff3359e56ffb1821928b 100644 (file)
--- a/api/gmxapi/cpp/context.cpp
+++ b/api/gmxapi/cpp/context.cpp
@@ -67,7 +67,7 @@
 #include "gromacs/utility/init.h"
 #include "gromacs/utility/smalloc.h"
 
-#include "gmxapi/mpi/multiprocessingresources.h"
+#include "gmxapi/mpi/resourceassignment.h"
 #include "gmxapi/exceptions.h"
 #include "gmxapi/session.h"
 #include "gmxapi/status.h"
@@ -89,9 +89,14 @@ namespace gmxapi
  */
 template<bool Value>
 using hasLibraryMpi = std::bool_constant<Value>;
-using gmxThreadMpi  = hasLibraryMpi<false>;
-using gmxLibMpi     = hasLibraryMpi<true>;
-using MpiType       = std::conditional_t<GMX_LIB_MPI, gmxLibMpi, gmxThreadMpi>;
+/* Note that a no-MPI build still uses the tMPI headers to define MPI_Comm for the
+ * gmx::SimulationContext definition. The dispatching in this file accounts for
+ * these two definitions of SimulationContext. gmxThreadMpi here does not imply
+ * that the library was necessarily compiled with thread-MPI enabled.
+ */
+using gmxThreadMpi = hasLibraryMpi<false>;
+using gmxLibMpi    = hasLibraryMpi<true>;
+using MpiType      = std::conditional_t<GMX_LIB_MPI, gmxLibMpi, gmxThreadMpi>;
 
 using MpiContextInitializationError = BasicException<struct MpiContextInitialization>;
 
@@ -228,7 +233,7 @@ ContextImpl::ContextImpl(MpiContextManager&& mpi) noexcept(std::is_nothrow_const
 {
     // Confirm our understanding of the MpiContextManager invariant.
     GMX_ASSERT(mpi_.communicator() == MPI_COMM_NULL ? !GMX_LIB_MPI : GMX_LIB_MPI,
-               "Precondition is an appropriate communicator for the library environment.");
+               "Precondition violated: inappropriate communicator for the library environment.");
     // Make sure we didn't change the data members and overlook implementation details.
     GMX_ASSERT(session_.expired(),
                "This implementation assumes an expired weak_ptr at initialization.");

diff --git a/api/gmxapi/cpp/context_impl.h b/api/gmxapi/cpp/context_impl.h
index 2f57c1404ef3014bdf1c94f0f0b0d067e20a2bcf..4f9296d379b59c52ffa2318e5ca110c782c6bb36 100644 (file)
--- a/api/gmxapi/cpp/context_impl.h
+++ b/api/gmxapi/cpp/context_impl.h
@@ -87,19 +87,25 @@ public:
      * Construct a valid instance with an appropriate default value for the
      * base communicator. (Note that appropriate default value depends on whether
      * the library was compiled with an external MPI library.)
+     *
+     * \throws BasicException if library cannot be initialized.
      */
     MpiContextManager();
 
     /*!
-     * \brief Borrow a communicator and ensure that MPI is initialized, if applicable.
-     *
-     * \param communicator Optional communicator representing a client-managed MPI environment.
+     * \brief Allow the communicator to be specified.
      *
+     * Supports use cases in which a client provides a non-default communicator.
+     * Ensures that library environment is properly initialized and finalized,
+     * whether or not an externally managed communicator has been provided.
      *
+     * \param communicator Optional communicator representing a client-managed MPI environment.
      *
      * Note that the communicator must be MPI_COMM_NULL if and only if GROMACS was built without an
      * external MPI library.
      *
+     * \throws BasicException if library cannot be initialized.
+     *
      * \todo (#3650?) Decide whether to exclude this from tMPI environments or find a sensible invariant.
      */
     explicit MpiContextManager(MPI_Comm communicator);
@@ -281,7 +287,18 @@ private:
     explicit ContextImpl(MpiContextManager&& mpi) noexcept(std::is_nothrow_constructible_v<gmx::LegacyMdrunOptions>);
 };
 
-
+/*!
+ * \brief Allow client code to hold a library communicator.
+ *
+ * MPI-enabled client code and thread-MPI GROMACS libraries cannot share headers
+ * in which MPI symbols (such as MPI_Comm) are defined. This class allows a
+ * client to refer to a library communicator opaquely. See gmxapi::offerComm().
+ *
+ * In the initial gmxapi::ResourceAssignment implementation, CommHandle objects
+ * are the only possible recipients of assigned resources. If future implementations
+ * allow richer resource assignment, CommHandle may be superseded by a more
+ * elaborate interface. Otherwise, all we need is this simple type proxy.
+ */
 class CommHandle
 {
 public:

diff --git a/api/gmxapi/cpp/multiprocessingresources.cpp b/api/gmxapi/cpp/resourceassignment.cpp
similarity index 98%
rename from api/gmxapi/cpp/multiprocessingresources.cpp
rename to api/gmxapi/cpp/resourceassignment.cpp
index 2c2f10711086a37a28f993d069b6fc688de676d2..877ce94aa354eed99308d866a4f4a35c6615c93d 100644 (file)
--- a/api/gmxapi/cpp/multiprocessingresources.cpp
+++ b/api/gmxapi/cpp/resourceassignment.cpp
@@ -42,7 +42,7 @@
  * \author "M. Eric Irrgang <ericirrgang@gmail.com"
  */
 
-#include "gmxapi/mpi/multiprocessingresources.h"
+#include "gmxapi/mpi/resourceassignment.h"
 
 #include "config.h"
 

diff --git a/api/gmxapi/include/gmxapi/mpi/gmxapi_mpi.h b/api/gmxapi/include/gmxapi/mpi/gmxapi_mpi.h
index b8f8db24d30b4bda6e082b4bb4cfb685e916b880..48ef5523559d325c28dc26197668d622a3f79595 100644 (file)
--- a/api/gmxapi/include/gmxapi/mpi/gmxapi_mpi.h
+++ b/api/gmxapi/include/gmxapi/mpi/gmxapi_mpi.h
@@ -43,7 +43,7 @@
 #include "gmxapi/context.h"
 #include "gmxapi/exceptions.h"
 
-#include "gmxapi/mpi/multiprocessingresources.h"
+#include "gmxapi/mpi/resourceassignment.h"
 
 /*! \file
  * \brief Provide details of any MPI implementation used when building the library.
@@ -83,12 +83,14 @@ public:
      * Create an abstract wrapper for client-provided values with which to initialize
      * simulation resources. When this wrapper is used, the client is responsible for providing
      * a valid communicator that will remain valid for the life of the consumer.
+     *
+     * \throws UsageError if client has not provided a valid communicator.
      */
     explicit ResourceAssignmentImpl(const CommT& communicator) : communicator_{ communicator }
     {
         if (communicator_ == MPI_COMM_NULL)
         {
-            throw UsageError("Null communicator cannot be lent.");
+            throw UsageError("Cannot assign a Null communicator.");
         }
         int flag = 0;
         MPI_Initialized(&flag);
@@ -100,7 +102,7 @@ public:
 
     [[nodiscard]] int size() const override
     {
-        assert(communicator_ != MPI_COMM_NULL && "Resource invariant implies a valid communicator.");
+        assert(communicator_ != MPI_COMM_NULL && "Class invariant broken: invalid communicator.");
         int size = 0;
         MPI_Comm_size(communicator_, &size);
         return size;
@@ -108,7 +110,7 @@ public:
 
     [[nodiscard]] int rank() const override
     {
-        assert(communicator_ != MPI_COMM_NULL && "Resource invariant implies a valid communicator.");
+        assert(communicator_ != MPI_COMM_NULL && "Class invariant broken: invalid communicator.");
         // This default value will never be read, but the compiler can't tell
         // that it is initialized by the MPI call.
         int rank = -1;
@@ -118,7 +120,7 @@ public:
 
     void applyCommunicator(CommHandle* dst) const override
     {
-        assert(communicator_ != MPI_COMM_NULL && "Resource invariant implies a valid communicator.");
+        assert(communicator_ != MPI_COMM_NULL && "Class invariant broken: invalid communicator.");
         offerComm(communicator_, dst);
     }
 
@@ -128,6 +130,9 @@ public:
 /*!
  * \brief Template header utility for connecting to MPI implementations.
  *
+ * The client provides a communicator for work executed within the scope of the Context.
+ * Client remains responsible for freeing the communicator and finalizing the MPI environment.
+ *
  * To use this helper function, the client software build environment must be
  * configured for an MPI implementation compatible with the target GROMACS library.
  *
@@ -144,14 +149,13 @@ public:
  *
  * \see createContext(std::unique_ptr<ResourceAssignment> resources)
  *
- * The client provides a communicator for work executed within the scope of the Context.
- * Client remains responsible for freeing the communicator and finalizing the MPI environment.
- *
  * The communicator resource type is a template parameter because MPI_Comm is commonly a C
  * typedef that varies between implementations, so we do not want to couple our
  * API to it, but we cannot forward-declare it.
  *
  * See also https://gitlab.com/gromacs/gromacs/-/issues/3650
+ *
+ * \throws UsageError if the provided resource is not usable.
  */
 template<typename CommT>
 std::unique_ptr<ResourceAssignment> assignResource(CommT communicator)

diff --git a/api/gmxapi/include/multiprocessingresources.cmakein.h b/api/gmxapi/include/resourceassignment.cmakein.h
similarity index 97%
rename from api/gmxapi/include/multiprocessingresources.cmakein.h
rename to api/gmxapi/include/resourceassignment.cmakein.h
index 8aa69d0906cabb86bc5ce3794170afb8ff2ab834..420b5958b1b4df3a9d5a43db9161dd4b3fead843 100644 (file)
--- a/api/gmxapi/include/multiprocessingresources.cmakein.h
+++ b/api/gmxapi/include/resourceassignment.cmakein.h
@@ -33,8 +33,8 @@
  * the research papers on the package. Check out http://www.gromacs.org.
  */
 
-#ifndef GMXAPI_MULTIPROCESSINGRESOURCES_H
-#define GMXAPI_MULTIPROCESSINGRESOURCES_H
+#ifndef GMXAPI_RESOURCEASSIGNMENT_H
+#define GMXAPI_RESOURCEASSIGNMENT_H
 
 /*! \file
  * \brief Provide build-specific overloads for client-MPI-dependent stuff.
@@ -132,4 +132,4 @@ void offerComm(MPI_Comm src, CommHandle* dst);
 
 } // end namespace gmxapi
 
-#endif // GMXAPI_MULTIPROCESSINGRESOURCES_H
+#endif // GMXAPI_RESOURCEASSIGNMENT_H