Relocate MpiContextManager.
authorM. Eric Irrgang <ericirrgang@gmail.com>
Mon, 28 Sep 2020 12:24:03 +0000 (15:24 +0300)
committerMark Abraham <mark.j.abraham@gmail.com>
Tue, 29 Sep 2020 06:09:40 +0000 (06:09 +0000)
* Change the scope of the MpiContextManager instance from the Session
  to the ContextImpl.
* Tighten up some details to help maintain clarity over subsequent
  changes.

Refs #3644

api/gmxapi/cpp/context.cpp
api/gmxapi/cpp/context_impl.h
api/gmxapi/cpp/session.cpp
api/gmxapi/cpp/session_impl.h

index 436ff90701acb53cd65c17522a1a7df5164a5916..c1555579a78148b52ce5e679a7f2b52ed81f34b9 100644 (file)
 #include "gromacs/commandline/pargs.h"
 #include "gromacs/commandline/filenm.h"
 #include "gromacs/commandline/pargs.h"
+#include "gromacs/gmxlib/network.h"
 #include "gromacs/mdlib/stophandler.h"
 #include "gromacs/mdrunutility/logging.h"
 #include "gromacs/mdrunutility/multisim.h"
 #include "gromacs/mdrun/runner.h"
 #include "gromacs/mdrunutility/handlerestart.h"
 #include "gromacs/utility/arraysize.h"
+#include "gromacs/utility/basenetwork.h"
 #include "gromacs/utility/fatalerror.h"
+#include "gromacs/utility/gmxmpi.h"
+#include "gromacs/utility/init.h"
 #include "gromacs/utility/smalloc.h"
 
+#include "gmxapi/mpi/multiprocessingresources.h"
 #include "gmxapi/exceptions.h"
 #include "gmxapi/session.h"
 #include "gmxapi/status.h"
 namespace gmxapi
 {
 
-ContextImpl::ContextImpl()
+MpiContextManager::MpiContextManager()
 {
+    gmx::init(nullptr, nullptr);
+    GMX_RELEASE_ASSERT(!GMX_LIB_MPI || gmx_mpi_initialized(),
+                       "MPI should be initialized before reaching this point.");
+};
+
+MpiContextManager::~MpiContextManager()
+{
+    // This is always safe to call. It is a no-op if
+    // thread-MPI, and if the constructor completed then the
+    // MPI library is initialized with reference counting.
+    gmx::finalize();
+}
+
+ContextImpl::~ContextImpl() = default;
+
+Context createContext()
+{
+    MpiContextManager contextmanager;
+    auto              impl = ContextImpl::create();
+    GMX_ASSERT(impl, "ContextImpl creation method should not be able to return null.");
+    auto context = Context(impl);
+    return context;
+}
+
+ContextImpl::ContextImpl() noexcept(std::is_nothrow_constructible_v<gmx::LegacyMdrunOptions>) :
+    mpi_(MpiContextManager())
+{
+    // 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.");
 }
 
-std::shared_ptr<gmxapi::ContextImpl> ContextImpl::create()
+std::shared_ptr<ContextImpl> ContextImpl::create()
 {
-    std::shared_ptr<ContextImpl> impl = std::make_shared<ContextImpl>();
+    std::shared_ptr<ContextImpl> impl;
+    impl.reset(new ContextImpl());
     return impl;
 }
 
@@ -164,14 +198,13 @@ std::shared_ptr<Session> ContextImpl::launch(const Workflow& work)
 
         ArrayRef<const std::string> multiSimDirectoryNames =
                 opt2fnsIfOptionSet("-multidir", ssize(options_.filenames), options_.filenames.data());
-        // Set up the communicator, where possible (see docs for
-        // SimulationContext).
-        MPI_Comm communicator = GMX_LIB_MPI ? MPI_COMM_WORLD : MPI_COMM_NULL;
+
         // The SimulationContext is necessary with gmxapi so that
         // resources owned by the client code can have suitable
         // lifetime. The gmx wrapper binary uses the same infrastructure,
         // but the lifetime is now trivially that of the invocation of the
         // wrapper binary.
+        auto              communicator = GMX_LIB_MPI ? MPI_COMM_WORLD : MPI_COMM_NULL;
         SimulationContext simulationContext(communicator, multiSimDirectoryNames);
 
 
index 0b5f9a69b2c3216de5b8f884535e4775e130016f..59acbaa67ec7c44138f8957889060c9cfb5e9c7c 100644 (file)
@@ -47,8 +47,6 @@
 #include "gromacs/mdrun/legacymdrunoptions.h"
 #include "gromacs/mdtypes/mdrunoptions.h"
 
-// Above are headers for dependencies.
-// Following are public headers for the current module.
 #include "gmxapi/context.h"
 #include "gmxapi/session.h"
 
@@ -56,26 +54,66 @@ namespace gmxapi
 {
 
 /*!
- * \brief Context implementation base class.
+ * \brief Provide RAII management of communications resource state.
  *
- * Execution contexts have a uniform interface specified by the API. Implementations for
- * particular execution environments can specialize / derive from this base.
+ * To acquire an MpiContextManager is to have assurance that any external MPI
+ * environment is ready to use. When the MpiContextManager is released or
+ * goes out of scope, the destructor finalizes the resources.
+ *
+ * Note that thread-MPI chooses the number of ranks and constructs its
+ * MPI communicator internally, so does not and is unlikely to ever
+ * participate here.
+ *
+ * \todo There is no resource for logging or reporting errors during initialization
  *
- * \todo Separate interface and implementation.
  * \ingroup gmxapi
  */
-class ContextImpl final : public std::enable_shared_from_this<ContextImpl>
+class MpiContextManager
 {
 public:
+    MpiContextManager();
+
+    ~MpiContextManager();
+
     /*!
-     * \brief Default constructor.
+     * \brief Exclusive ownership of a scoped context means copying is impossible.
      *
-     * Don't use this. Use create() to get a shared pointer right away.
-     * Otherwise, shared_from_this() is potentially dangerous.
+     * \{
+     */
+    MpiContextManager(const MpiContextManager&) = delete;
+    MpiContextManager& operator=(const MpiContextManager&) = delete;
+    //! \}
+
+    /*!
+     * \brief Move semantics are non-trivial.
      *
-     * \todo Make default constructor private or otherwise reduce brittleness of construction.
+     * \{
      */
-    ContextImpl();
+    MpiContextManager(MpiContextManager&&) noexcept = delete;
+    MpiContextManager& operator=(MpiContextManager&&) noexcept = delete;
+    //! \}
+};
+
+/*!
+ * \brief Context implementation.
+ *
+ * Execution contexts have a uniform interface specified by the API. Implementations for
+ * particular execution environments can specialize / derive from this base.
+ *
+ * \todo Separate interface and implementation.
+ *
+ * \warning Definition and semantics depend on configure-time details. For example,
+ *          MPI-enabled libraries always hold a valid MPI communicator via MpiContextManager,
+ *          whereas tMPI and non-MPI builds hold a meaningless MpiContextManager.
+ *
+ * \todo Provide functions or traits for introspection.
+ *
+ * \ingroup gmxapi
+ */
+class ContextImpl final : public std::enable_shared_from_this<ContextImpl>
+{
+public:
+    ~ContextImpl();
 
     /*!
      * \brief Factory function
@@ -84,10 +122,12 @@ public:
      * that it never exists without a shared_ptr owning it.
      *
      * If we can confirm `shared_from_this` is no longer necessary, implementation may change.
+     * \todo: Use registration/deregistration of launched Sessions to log warnings on shutdown
+     *        instead of letting Session keep ContextImpl alive.
      *
      * \return ownership of a new object
      */
-    static std::shared_ptr<gmxapi::ContextImpl> create();
+    static std::shared_ptr<ContextImpl> create();
 
     /*!
      * \brief Copy disallowed because Session state would become ambiguous.
@@ -103,7 +143,9 @@ public:
     /*!
      * \brief Objects are not trivial to move.
      *
-     * \todo Implement move semantics.
+     * \todo Implement move semantics. Requires a moveable const MpiContextManager and
+     *       LegacyMdrunOptions members.
+     *
      * \{
      */
     ContextImpl(ContextImpl&&) = delete;
@@ -123,6 +165,8 @@ public:
      * are still evolving.
      * \todo Hide lifetime management and ownership from handle object.
      * We can achieve the necessary aspects of this shared_ptr at a lower level of implementation.
+     * Note also that returned value policies can be implemented in higher level wrappers to ensure
+     * correct object lifetime scope. (See pybind, for instance.)
      */
     std::shared_ptr<Session> launch(const Workflow& work);
 
@@ -131,6 +175,9 @@ public:
      *
      * The client owns the Session launched by a Context, but it is helpful
      * for the Context to know if it has an active Session associated with it.
+     *
+     * \todo Use registration/deregistration protocol instead.
+     *       Requires logging facility.
      */
     std::weak_ptr<Session> session_;
 
@@ -152,6 +199,34 @@ public:
      * duplicate definitions e.g. of command-line options.
      */
     gmx::LegacyMdrunOptions options_;
+
+    /*!
+     * \brief Scoped MPI management.
+     *
+     * Part of the ContextImpl invariant establishes a point where MPI initialization status is
+     * known.
+     *
+     * To ensure the MpiContextManager is initialized only once, we use a const member that must
+     * be initialized at construction.
+     */
+    const MpiContextManager mpi_;
+
+private:
+    /*!
+     * \brief Basic constructor.
+     *
+     * Don't use this. Use create() to get a shared pointer right away.
+     * Otherwise, shared_from_this() is potentially dangerous.
+     */
+    explicit ContextImpl() noexcept(std::is_nothrow_constructible_v<gmx::LegacyMdrunOptions>);
+};
+
+
+class CommHandle
+{
+public:
+    using commType = MPI_Comm;
+    MPI_Comm communicator{ MPI_COMM_NULL };
 };
 
 } // end namespace gmxapi
index 5a9d0f274a32374e0d70599ce1a66a66a5fbbd75..2b909663b094a2d9b579957fb6bccbffe6a333cd 100644 (file)
 namespace gmxapi
 {
 
-/*!
- * \brief Provide RAII management of communications resource state.
- *
- * To acquire an MpiContextManager is to have assurance that any external MPI
- * environment is ready to use. When the MpiContextManager is released or
- * goes out of scope, the destructor finalizes the resources.
- *
- * Note that thread-MPI chooses the number of ranks and constructs its
- * MPI communicator internally, so does not and is unlikely to ever
- * participate here.
- *
- * \todo There is no resource for logging or reporting errors during initialization
- * \todo Remove this class by managing the MPI context with mpi4py and so
- *       configuring the SimulationContext externally
- *
- * \ingroup gmxapi
- */
-class MpiContextManager
-{
-public:
-    MpiContextManager()
-    {
-        gmx::init(nullptr, nullptr);
-        GMX_RELEASE_ASSERT(!GMX_LIB_MPI || gmx_mpi_initialized(),
-                           "MPI should be initialized before reaching this point.");
-    };
-
-    ~MpiContextManager()
-    {
-        // This is always safe to call. It is a no-op if
-        // thread-MPI, and if the constructor completed then the
-        // MPI library is initialized with reference counting.
-        gmx::finalize();
-    }
-
-    /*!
-     * \brief Exclusive ownership of a scoped context means copying is impossible.
-     *
-     * \{
-     */
-    MpiContextManager(const MpiContextManager&) = delete;
-    MpiContextManager& operator=(const MpiContextManager&) = delete;
-    //! \}
-
-    /*!
-     * \brief Move semantics are trivial.
-     *
-     * \{
-     */
-    MpiContextManager(MpiContextManager&&) noexcept = default;
-    MpiContextManager& operator=(MpiContextManager&&) noexcept = default;
-    //! \}
-};
-
 SignalManager::SignalManager(gmx::StopHandlerBuilder* stopHandlerBuilder) :
     state_(std::make_shared<gmx::StopSignal>(gmx::StopSignal::noSignal))
 {
@@ -185,12 +131,10 @@ SessionImpl::SessionImpl(std::shared_ptr<ContextImpl> context,
                          gmx::SimulationContext&&     simulationContext,
                          gmx::LogFilePtr              fplog) :
     context_(std::move(context)),
-    mpiContextManager_(std::make_unique<MpiContextManager>()),
     simulationContext_(std::move(simulationContext)),
     logFilePtr_(std::move(fplog))
 {
     GMX_ASSERT(context_, "SessionImpl invariant implies valid ContextImpl handle.");
-    GMX_ASSERT(mpiContextManager_, "SessionImpl invariant implies valid MpiContextManager guard.");
 
     // \todo Session objects can have logic specialized for the runtime environment.
 
index 9d07ac7cb2165bfe0b7dd621e2566f5a1b056fb8..3d9699e5cf06765d1565fe09be312dac32052b98 100644 (file)
@@ -60,9 +60,8 @@ namespace gmxapi
 {
 
 // Forward declaration
-class MpiContextManager; // Locally defined in session.cpp
-class ContextImpl;       // locally defined in context.cpp
-class SignalManager;     // defined in mdsignals_impl.h
+class ContextImpl;   // locally defined in context.cpp
+class SignalManager; // defined in mdsignals_impl.h
 
 /*!
  * \brief Implementation class for executing sessions.
@@ -202,18 +201,12 @@ private:
      * \brief Extend the life of the owning context.
      *
      * The session will get handles for logging, UI status messages,
-     * and other facilities through this interface.
+     * and other facilities through this interface. This is a facility
+     * provided by the client to the Session implementation during
+     * Context.launch().
      */
     std::shared_ptr<ContextImpl> context_;
 
-    /*!
-     * \brief RAII management of gmx::init() and gmx::finalize()
-     *
-     * Uses smart pointer to avoid exposing type definition.
-     * \todo Not fully implemented.
-     */
-    std::unique_ptr<MpiContextManager> mpiContextManager_;
-
     /*!
      * \brief Simulation runner object.
      *
@@ -224,6 +217,10 @@ private:
 
     /*!
      * \brief An active session owns the resources it is using.
+     *
+     * This encapsulate details of the run time context that the
+     * Session makes available to the simulator, tied to the
+     * lifetime of the Session.
      */
     gmx::SimulationContext simulationContext_;