Add strict assertions on x synchronizer in PME
authorSzilárd Páll <pall.szilard@gmail.com>
Tue, 15 Oct 2019 10:57:39 +0000 (12:57 +0200)
committerBerk Hess <hess@kth.se>
Fri, 18 Oct 2019 21:06:30 +0000 (23:06 +0200)
To be able to add strict assertions on when a valid event synchronizer
needs to be passed to PME, a new booean is stored in the PME GPU data
structures indicating whether execution is happening on a separate PME
rank.

Also clarified some function argument doxygen.

Change-Id: Ic141c70dded2b57f39b7f2e2bfa1a17d80604204

src/gromacs/ewald/pme.h
src/gromacs/ewald/pme_gpu.cpp
src/gromacs/ewald/pme_gpu_internal.cpp
src/gromacs/ewald/pme_gpu_types_host.h

index 202195997748cd8600d124a22018f0ec394a5ea6..92135a282371c04c24440fac05e12eb87ef76e6f 100644 (file)
@@ -370,7 +370,7 @@ GPU_FUNC_QUALIFIER void pme_gpu_prepare_computation(gmx_pme_t      *GPU_FUNC_ARG
  * Launches first stage of PME on GPU - spreading kernel.
  *
  * \param[in] pme                The PME data structure.
- * \param[in] xReadyOnDevice     Event synchronizer indicating that the coordinates are ready in the device memory.
+ * \param[in] xReadyOnDevice     Event synchronizer indicating that the coordinates are ready in the device memory; nullptr allowed only on separate PME ranks.
  * \param[in] wcycle             The wallclock counter.
  */
 GPU_FUNC_QUALIFIER void pme_gpu_launch_spread(gmx_pme_t            *GPU_FUNC_ARGUMENT(pme),
index 8bbd184ac8a48d97e98e034c6c1415b7dcae504f..565a63946a1cfee711bc9316bdaf5978b2ccbb42 100644 (file)
@@ -179,6 +179,7 @@ void pme_gpu_launch_spread(gmx_pme_t            *pme,
                            gmx_wallcycle        *wcycle)
 {
     GMX_ASSERT(pme_gpu_active(pme), "This should be a GPU run of PME but it is not enabled.");
+    GMX_ASSERT(xReadyOnDevice || !pme->bPPnode || (GMX_GPU != GMX_GPU_CUDA), "Need a valid xReadyOnDevice on PP+PME ranks with CUDA.");
 
     PmeGpu            *pmeGpu = pme->gpu;
 
index e98ae3a223c8be7ee88cd7aad5682b32866f2442..a3a25c408b027815ff9123c7d5d6b7edc38231de 100644 (file)
@@ -790,8 +790,9 @@ static void pme_gpu_copy_common_data_from(const gmx_pme_t *pme)
     pmeGpu->common->nn.insert(pmeGpu->common->nn.end(), pme->nnx, pme->nnx + cellCount * pme->nkx);
     pmeGpu->common->nn.insert(pmeGpu->common->nn.end(), pme->nny, pme->nny + cellCount * pme->nky);
     pmeGpu->common->nn.insert(pmeGpu->common->nn.end(), pme->nnz, pme->nnz + cellCount * pme->nkz);
-    pmeGpu->common->runMode   = pme->runMode;
-    pmeGpu->common->boxScaler = pme->boxScaler;
+    pmeGpu->common->runMode       = pme->runMode;
+    pmeGpu->common->isRankPmeOnly = !pme->bPPnode;
+    pmeGpu->common->boxScaler     = pme->boxScaler;
 }
 
 /*! \libinternal \brief
@@ -1041,8 +1042,9 @@ void pme_gpu_spread(const PmeGpu         *pmeGpu,
     // Ensure that coordinates are ready on the device before launching spread;
     // only needed with CUDA on PP+PME ranks, not on separate PME ranks, in unit tests
     // nor in OpenCL as these cases use a single stream (hence xReadyOnDevice == nullptr).
-    // Note: Consider adding an assertion on xReadyOnDevice when we can detect
-    // here separate PME ranks.
+    GMX_ASSERT(xReadyOnDevice != nullptr ||
+               (GMX_GPU != GMX_GPU_CUDA) || pmeGpu->common->isRankPmeOnly || pme_gpu_is_testing(pmeGpu),
+               "Need a valid coordinate synchronizer on PP+PME ranks with CUDA.");
     if (xReadyOnDevice)
     {
         xReadyOnDevice->enqueueWaitEvent(pmeGpu->archSpecific->pmeStream);
index baf648fbc3a9ec170adc0871bb1dbb9b3c7d43d0..a35be1e8841e19eb9a259b01d657d0483c5e1ed6 100644 (file)
@@ -179,6 +179,8 @@ struct PmeShared
     std::vector<real>      bsp_mod[DIM];
     /*! \brief The PME codepath being taken */
     PmeRunMode             runMode;
+    /*! \brief  Whether PME execution is happening on a PME-only rank (from gmx_pme_t.bPPnode). */
+    bool                   isRankPmeOnly;
     /*! \brief The box scaler based on inputrec - created in pme_init and managed by CPU structure */
     class EwaldBoxZScaler *boxScaler;
     /*! \brief The previous computation box to know if we even need to update the current box params.