Avoid unnecessary exclusion generation in DD

With only self exclusions, the DD partitioning would generate a list
of all empty exclusion lists for all i-atoms. Now no lists are
generated. An assertion is added in the pairlist generation code
to ensure we either have 0 or the right number of exclusions lists.

Change-Id: I4b0556cbf3ed3a6fea7bdd0e03e194410bcfb88a

diff --git a/src/gromacs/domdec/domdec_topology.cpp b/src/gromacs/domdec/domdec_topology.cpp
index 8cc5430000bd636be847a151888b6d2a0a040a87..e8fa082df4d489dafe5119cb261673fe4782541c 100644 (file)
--- a/src/gromacs/domdec/domdec_topology.cpp
+++ b/src/gromacs/domdec/domdec_topology.cpp
@@ -1340,15 +1340,6 @@ static int make_bondeds_zone(gmx_domdec_t*                      dd,
     return nbonded_local;
 }
 
-/*! \brief Set the exclusion data for i-zone \p iz for the case of no exclusions */
-static void set_no_exclusions_zone(const gmx_domdec_zones_t* zones, int iz, ListOfLists<int>* lexcls)
-{
-    for (int a = zones->cg_range[iz]; a < zones->cg_range[iz + 1]; a++)
-    {
-        lexcls->pushBack({});
-    }
-}
-
 /*! \brief Set the exclusion data for i-zone \p iz */
 static void make_exclusions_zone(gmx_domdec_t*                     dd,
                                  gmx_domdec_zones_t*               zones,
@@ -1420,24 +1411,6 @@ static void make_exclusions_zone(gmx_domdec_t*                     dd,
             "The number of exclusion list should match the number of atoms in the range");
 }
 
-/*! \brief Set the total count indexes for the local exclusions, needed by several functions */
-static void finish_local_exclusions(gmx_domdec_t* dd, gmx_domdec_zones_t* zones, ListOfLists<int>* lexcls)
-{
-    const gmx::Range<int> nonhomeIzonesAtomRange(zones->iZones[0].iAtomRange.end(),
-                                                 zones->iZones.back().iAtomRange.end());
-
-    if (!dd->haveExclusions)
-    {
-        /* There are no exclusions involving non-home charge groups,
-         * but we need to set the indices for neighborsearching.
-         */
-        for (int gmx_unused la : nonhomeIzonesAtomRange)
-        {
-            lexcls->pushBack({});
-        }
-    }
-}
-
 /*! \brief Clear a t_idef struct */
 static void clear_idef(t_idef* idef)
 {
@@ -1465,7 +1438,7 @@ static int make_local_bondeds_excls(gmx_domdec_t*       dd,
                                     ListOfLists<int>*   lexcls,
                                     int*                excl_count)
 {
-    int                nzone_bondeds, nzone_excl;
+    int                nzone_bondeds;
     int                cg0, cg1;
     real               rc2;
     int                nbonded_local;
@@ -1483,16 +1456,8 @@ static int make_local_bondeds_excls(gmx_domdec_t*       dd,
         nzone_bondeds = 1;
     }
 
-    if (dd->haveExclusions)
-    {
-        /* We only use exclusions from i-zones to i- and j-zones */
-        nzone_excl = zones->iZones.size();
-    }
-    else
-    {
-        /* There are no exclusions and only zone 0 sees itself */
-        nzone_excl = 1;
-    }
+    /* We only use exclusions from i-zones to i- and j-zones */
+    const int numIZonesForExclusions = (dd->haveExclusions ? zones->iZones.size() : 0);
 
     rt = dd->reverse_top;
 
@@ -1536,7 +1501,7 @@ static int make_local_bondeds_excls(gmx_domdec_t*       dd,
                         dd, zones, mtop->molblock, bRCheckMB, rcheck, bRCheck2B, rc2, pbc_null,
                         cg_cm, idef->iparams, idef_t, izone, gmx::Range<int>(cg0t, cg1t));
 
-                if (izone < nzone_excl)
+                if (izone < numIZonesForExclusions)
                 {
                     ListOfLists<int>* excl_t;
                     if (thread == 0)
@@ -1569,7 +1534,7 @@ static int make_local_bondeds_excls(gmx_domdec_t*       dd,
             nbonded_local += th_work.nbonded;
         }
 
-        if (izone < nzone_excl)
+        if (izone < numIZonesForExclusions)
         {
             for (std::size_t th = 1; th < rt->th_work.size(); th++)
             {
@@ -1582,15 +1547,6 @@ static int make_local_bondeds_excls(gmx_domdec_t*       dd,
         }
     }
 
-    /* Some zones might not have exclusions, but some code still needs to
-     * loop over the index, so we set the indices here.
-     */
-    for (size_t iZone = nzone_excl; iZone < zones->iZones.size(); iZone++)
-    {
-        set_no_exclusions_zone(zones, iZone, lexcls);
-    }
-
-    finish_local_exclusions(dd, zones, lexcls);
     if (debug)
     {
         fprintf(debug, "We have %d exclusions, check count %d\n", lexcls->numElements(), *excl_count);

diff --git a/src/gromacs/nbnxm/nbnxm.h b/src/gromacs/nbnxm/nbnxm.h
index bd63c4c6e2551e35ed0060808bbaa8a533f49d3e..1a27e2e160dd06cbf290570cb0898cf7cea373dc 100644 (file)
--- a/src/gromacs/nbnxm/nbnxm.h
+++ b/src/gromacs/nbnxm/nbnxm.h
@@ -252,7 +252,18 @@ public:
     //! Returns the index position of the atoms on the search grid
     gmx::ArrayRef<const int> getGridIndices() const;
 
-    //! Constructs the pairlist for the given locality
+    /*! \brief Constructs the pairlist for the given locality
+     *
+     * When there are no non-self exclusions, \p exclusions can be empty.
+     * Otherwise the number of lists in \p exclusions should match the number
+     * of atoms when not using DD, or the total number of atoms in the i-zones
+     * when using DD.
+     *
+     * \param[in] iLocality   The interaction locality: local or non-local
+     * \param[in] exclusions  Lists of exclusions for every atom.
+     * \param[in] step        Used to set the list creation step
+     * \param[in,out] nrnb    Flop accounting struct, can be nullptr
+     */
     void constructPairlist(gmx::InteractionLocality     iLocality,
                            const gmx::ListOfLists<int>& exclusions,
                            int64_t                      step,

diff --git a/src/gromacs/nbnxm/pairlist.cpp b/src/gromacs/nbnxm/pairlist.cpp
index 955c0967e6c4a273fdaacefcb9a321d107326079..6dee69782e17ecd717a96b09e2b2b345dd389883 100644 (file)
--- a/src/gromacs/nbnxm/pairlist.cpp
+++ b/src/gromacs/nbnxm/pairlist.cpp
@@ -3539,9 +3539,12 @@ static void nbnxn_make_pairlist_part(const Nbnxm::GridSet&   gridSet,
                         }
                     }
 
-                    /* Set the exclusions for this ci list */
-                    setExclusionsForIEntry(gridSet, nbl, excludeSubDiagonal, na_cj_2log,
-                                           *getOpenIEntry(nbl), exclusions);
+                    if (!exclusions.empty())
+                    {
+                        /* Set the exclusions for this ci list */
+                        setExclusionsForIEntry(gridSet, nbl, excludeSubDiagonal, na_cj_2log,
+                                               *getOpenIEntry(nbl), exclusions);
+                    }
 
                     if (haveFep)
                     {
@@ -4201,8 +4204,20 @@ void PairlistSets::construct(const InteractionLocality iLocality,
                              const int64_t             step,
                              t_nrnb*                   nrnb)
 {
-    pairlistSet(iLocality).constructPairlists(pairSearch->gridSet(), pairSearch->work(), nbat,
-                                              exclusions, minimumIlistCountForGpuBalancing_, nrnb,
+    const auto& gridSet = pairSearch->gridSet();
+    const auto* ddZones = gridSet.domainSetup().zones;
+
+    /* The Nbnxm code can also work with more exclusions than those in i-zones only
+     * when using DD, but the equality check can catch more issues.
+     */
+    GMX_RELEASE_ASSERT(
+            exclusions.empty() || (!ddZones && exclusions.ssize() == gridSet.numRealAtomsTotal())
+                    || (ddZones && exclusions.ssize() == ddZones->cg_range[ddZones->iZones.size()]),
+            "exclusions should either be empty or the number of lists should match the number of "
+            "local i-atoms");
+
+    pairlistSet(iLocality).constructPairlists(gridSet, pairSearch->work(), nbat, exclusions,
+                                              minimumIlistCountForGpuBalancing_, nrnb,
                                               &pairSearch->cycleCounting_);
 
     if (iLocality == InteractionLocality::Local)