Do not allow AtomLocality::All for coordinates and velocities transfer.

1. Use AtomLocality:Local instead of AtomLocality::All in PME-only ranks
   and in PME tests.
2. Assert on using AtomLocality::All for positions and velocities transfers,
   print verbose message on assert.

This reduces the number of possible streams used in StatePropagatoerData, hence
reduces its complexity. In preparation for buffer state tracking.

Refs. #3323

Refs. #3020

diff --git a/src/gromacs/ewald/pme_only.cpp b/src/gromacs/ewald/pme_only.cpp
index 0e67991867eb750d8bb499a27b24887d84236b33..8e33845218145d6a0b85ff6138c275301cc3bb02 100644 (file)
--- a/src/gromacs/ewald/pme_only.cpp
+++ b/src/gromacs/ewald/pme_only.cpp
@@ -759,7 +759,8 @@ int gmx_pmeonly(struct gmx_pme_t*               pme,
             pme_gpu_prepare_computation(pme, box, wcycle, stepWork);
             if (!pme_pp->useGpuDirectComm)
             {
-                stateGpu->copyCoordinatesToGpu(gmx::ArrayRef<gmx::RVec>(pme_pp->x), gmx::AtomLocality::All);
+                stateGpu->copyCoordinatesToGpu(gmx::ArrayRef<gmx::RVec>(pme_pp->x),
+                                               gmx::AtomLocality::Local);
             }
             // On the separate PME rank we do not need a synchronizer as we schedule everything in a single stream
             // TODO: with pme on GPU the receive should make a list of synchronizers and pass it here #3157

diff --git a/src/gromacs/ewald/tests/pmetestcommon.cpp b/src/gromacs/ewald/tests/pmetestcommon.cpp
index 3d611ca0d0ea3552232cc5d404c1fcf5487ed10b..3eda383e7acfc6986df140f789f7ca817ae175be 100644 (file)
--- a/src/gromacs/ewald/tests/pmetestcommon.cpp
+++ b/src/gromacs/ewald/tests/pmetestcommon.cpp
@@ -212,7 +212,7 @@ void pmeInitAtoms(gmx_pme_t*               pme,
 
             stateGpu->reinit(atomCount, atomCount);
             stateGpu->copyCoordinatesToGpu(arrayRefFromArray(coordinates.data(), coordinates.size()),
-                                           gmx::AtomLocality::All);
+                                           gmx::AtomLocality::Local);
             pme_gpu_set_kernelparam_coordinates(pme->gpu, stateGpu->getCoordinates());
 
             break;

diff --git a/src/gromacs/mdtypes/locality.h b/src/gromacs/mdtypes/locality.h
index 709fcfa3ace519afedc06a7cada6a1135126f41f..0ddfa883ea19acd8860c2894e8176de56533e714 100644 (file)
--- a/src/gromacs/mdtypes/locality.h
+++ b/src/gromacs/mdtypes/locality.h
@@ -62,11 +62,17 @@ enum class AtomLocality : int
     Count    = 3  //!< The number of atom locality types
 };
 
-/*! \brief Descriptive strings for atom localities */
-static const EnumerationArray<AtomLocality, const char*> c_atomLocalityNames = {
-    { "local", "non-local", "all" }
-};
-
+/*! \brief Get the human-friendly name for atom localities.
+ *
+ * \param[in] enumValue The enum value to get the name for.
+ */
+[[maybe_unused]] static const char* enumValueToString(AtomLocality enumValue)
+{
+    static constexpr gmx::EnumerationArray<AtomLocality, const char*> atomLocalityNames = {
+        "Local", "Non-local", "All"
+    };
+    return atomLocalityNames[enumValue];
+}
 /*! \brief Interaction locality indicator: local, non-local, all.
  *
  * Used for calls to:
@@ -79,10 +85,17 @@ enum class InteractionLocality : int
     Count    = 2  //!< The number of interaction locality types
 };
 
-/*! \brief Descriptive strings for interaction localities */
-static const EnumerationArray<InteractionLocality, const char*> c_interactionLocalityNames = {
-    { "local", "non-local" }
-};
+/*! \brief Get the human-friendly name for interaction localities.
+ *
+ * \param[in] enumValue The enum value to get the name for.
+ */
+[[maybe_unused]] static const char* enumValueToString(InteractionLocality enumValue)
+{
+    static constexpr gmx::EnumerationArray<InteractionLocality, const char*> interactionLocalityNames = {
+        "Local", "Non-local"
+    };
+    return interactionLocalityNames[enumValue];
+}
 
 /*! \brief Convert atom locality to interaction locality.
  *

diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp b/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp
index 93a0839b9e617007bcad3fbf47296dca6d58ab97..28d60f86133fff57ceb6db3706aa78bee0a7e86d 100644 (file)
--- a/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp
+++ b/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp
@@ -85,11 +85,11 @@ StatePropagatorDataGpu::Impl::Impl(const DeviceStreamManager& deviceStreamManage
     // Note, that nullptr stream is used here to indicate that the copy is not supported.
     xCopyStreams_[AtomLocality::Local]    = updateStream_;
     xCopyStreams_[AtomLocality::NonLocal] = nonLocalStream_;
-    xCopyStreams_[AtomLocality::All]      = updateStream_;
+    xCopyStreams_[AtomLocality::All]      = nullptr;
 
     vCopyStreams_[AtomLocality::Local]    = updateStream_;
     vCopyStreams_[AtomLocality::NonLocal] = nullptr;
-    vCopyStreams_[AtomLocality::All]      = updateStream_;
+    vCopyStreams_[AtomLocality::All]      = nullptr;
 
     fCopyStreams_[AtomLocality::Local]    = localStream_;
     fCopyStreams_[AtomLocality::NonLocal] = nonLocalStream_;
@@ -121,7 +121,7 @@ StatePropagatorDataGpu::Impl::Impl(const DeviceStream*  pmeStream,
     // This it temporary measure to make it safe to use this class in those cases.
     xCopyStreams_[AtomLocality::Local]    = pmeStream_;
     xCopyStreams_[AtomLocality::NonLocal] = nullptr;
-    xCopyStreams_[AtomLocality::All]      = pmeStream_;
+    xCopyStreams_[AtomLocality::All]      = nullptr;
 
     vCopyStreams_[AtomLocality::Local]    = nullptr;
     vCopyStreams_[AtomLocality::NonLocal] = nullptr;
@@ -311,7 +311,12 @@ DeviceBuffer<RVec> StatePropagatorDataGpu::Impl::getCoordinates()
 void StatePropagatorDataGpu::Impl::copyCoordinatesToGpu(const gmx::ArrayRef<const gmx::RVec> h_x,
                                                         AtomLocality atomLocality)
 {
-    GMX_ASSERT(atomLocality < AtomLocality::Count, "Wrong atom locality.");
+    GMX_ASSERT(atomLocality < AtomLocality::All,
+               formatString("Wrong atom locality. Only Local and NonLocal are allowed for "
+                            "coordinate transfers, passed value is \"%s\"",
+                            enumValueToString(atomLocality))
+                       .c_str());
+
     const DeviceStream* deviceStream = xCopyStreams_[atomLocality];
     GMX_ASSERT(deviceStream != nullptr,
                "No stream is valid for copying positions with given atom locality.");
@@ -376,7 +381,11 @@ GpuEventSynchronizer* StatePropagatorDataGpu::Impl::xUpdatedOnDevice()
 
 void StatePropagatorDataGpu::Impl::copyCoordinatesFromGpu(gmx::ArrayRef<gmx::RVec> h_x, AtomLocality atomLocality)
 {
-    GMX_ASSERT(atomLocality < AtomLocality::Count, "Wrong atom locality.");
+    GMX_ASSERT(atomLocality < AtomLocality::All,
+               formatString("Wrong atom locality. Only Local and NonLocal are allowed for "
+                            "coordinate transfers, passed value is \"%s\"",
+                            enumValueToString(atomLocality))
+                       .c_str());
     const DeviceStream* deviceStream = xCopyStreams_[atomLocality];
     GMX_ASSERT(deviceStream != nullptr,
                "No stream is valid for copying positions with given atom locality.");
@@ -408,7 +417,11 @@ DeviceBuffer<RVec> StatePropagatorDataGpu::Impl::getVelocities()
 void StatePropagatorDataGpu::Impl::copyVelocitiesToGpu(const gmx::ArrayRef<const gmx::RVec> h_v,
                                                        AtomLocality atomLocality)
 {
-    GMX_ASSERT(atomLocality < AtomLocality::Count, "Wrong atom locality.");
+    GMX_ASSERT(atomLocality == AtomLocality::Local,
+               formatString("Wrong atom locality. Only Local is allowed for "
+                            "velocity transfers, passed value is \"%s\"",
+                            enumValueToString(atomLocality))
+                       .c_str());
     const DeviceStream* deviceStream = vCopyStreams_[atomLocality];
     GMX_ASSERT(deviceStream != nullptr,
                "No stream is valid for copying velocities with given atom locality.");
@@ -428,7 +441,11 @@ void StatePropagatorDataGpu::Impl::copyVelocitiesToGpu(const gmx::ArrayRef<const
 
 void StatePropagatorDataGpu::Impl::copyVelocitiesFromGpu(gmx::ArrayRef<gmx::RVec> h_v, AtomLocality atomLocality)
 {
-    GMX_ASSERT(atomLocality < AtomLocality::Count, "Wrong atom locality.");
+    GMX_ASSERT(atomLocality == AtomLocality::Local,
+               formatString("Wrong atom locality. Only Local is allowed for "
+                            "velocity transfers, passed value is \"%s\"",
+                            enumValueToString(atomLocality))
+                       .c_str());
     const DeviceStream* deviceStream = vCopyStreams_[atomLocality];
     GMX_ASSERT(deviceStream != nullptr,
                "No stream is valid for copying velocities with given atom locality.");