Fixed NSGrid (#3133)

* Fixed NSGrid Rewrote lib.nsgrid module Issues #2919 #2345 #2229 * enabled nsgrid again * bumped version to 1.0.2-dev * remove nsgrid warnings * regenerated nsgrid C files * Revert removal of nsgrid from test_distances Co-authored-by: IAlibay <irfan.alibay@gmail.com>
MDAnalysis · Mar 7, 2021 · d9dcdcd · d9dcdcd
1 parent bc95e31
commit d9dcdcd
Show file tree

Hide file tree

Showing 14 changed files with 9,484 additions and 13,817 deletions.
diff --git a/package/CHANGELOG b/package/CHANGELOG
@@ -13,6 +13,14 @@ The rules for this file:
   * release numbers follow "Semantic Versioning" http://semver.org
 
 ------------------------------------------------------------------------------
+??/??/??  richardjgowers
+
+  * 1.0.2
+
+Fixes
+  * Fixed several issues with NSGrid and triclinic boxes not finding some pairs.
+    (Issues #2229 #2345 #2919) PR #2937.
+
 01/17/21 richardjgowers, IAlibay, orbeckst, tylerjereddy, jbarnoud,
          yuxuanzhuang, lilyminium, VOD555, p-j-smith, bieniekmateusz,
          calcraven, ianmkenney, rcrehuet, manuel.nuno.melo, hanatok

diff --git a/package/MDAnalysis/lib/distances.py b/package/MDAnalysis/lib/distances.py
@@ -400,6 +400,8 @@ def capped_distance(reference, configuration, max_cutoff, min_cutoff=None,
        nsgrid was temporarily removed and replaced with pkdtree due to issues
        relating to its reliability and accuracy (Issues #2919, #2229, #2345,
        #2670, #2930)
+    .. versionchanged:: 1.0.2
+       nsgrid enabled again
     """
     if box is not None:
         box = np.asarray(box, dtype=np.float32)
@@ -449,10 +451,13 @@ def _determine_method(reference, configuration, max_cutoff, min_cutoff=None,
        nsgrid was temporarily removed and replaced with pkdtree due to issues
        relating to its reliability and accuracy (Issues #2919, #2229, #2345,
        #2670, #2930)
+    .. versionchanged:: 1.0.2
+       enabled nsgrid again
     """
-    # TODO: add 'nsgrid': _nsgrid_capped back once fixed
     methods = {'bruteforce': _bruteforce_capped,
-               'pkdtree': _pkdtree_capped}
+               'pkdtree': _pkdtree_capped,
+               'nsgrid': _nsgrid_capped,
+    }
 
     if method is not None:
         return methods[method.lower()]
@@ -462,8 +467,7 @@ def _determine_method(reference, configuration, max_cutoff, min_cutoff=None,
     elif len(reference) * len(configuration) >= 1e8:
         # CAUTION : for large datasets, shouldnt go into 'bruteforce'
         # in any case. Arbitrary number, but can be characterized
-        # Temporarily replace nsgrid with pkdtree Issue #2930
-        return methods['pkdtree']
+        return methods['nsgrid']
     else:
         if box is None:
             min_dim = np.array([reference.min(axis=0),
@@ -479,8 +483,7 @@ def _determine_method(reference, configuration, max_cutoff, min_cutoff=None,
         if np.any(max_cutoff > 0.3*size):
             return methods['bruteforce']
         else:
-            # Temporarily replace nsgrid with pkdtree Issue #2930
-            return methods['pkdtree']
+            return methods['nsgrid']
 
 
 @check_coords('reference', 'configuration', enforce_copy=False,
@@ -817,6 +820,8 @@ def self_capped_distance(reference, max_cutoff, min_cutoff=None, box=None,
        nsgrid was temporarily removed and replaced with pkdtree due to issues
        relating to its reliability and accuracy (Issues #2919, #2229, #2345,
        #2670, #2930)
+    .. versionchanged:: 1.0.2
+       enabled nsgrid again
     """
     if box is not None:
         box = np.asarray(box, dtype=np.float32)
@@ -864,10 +869,13 @@ def _determine_method_self(reference, max_cutoff, min_cutoff=None, box=None,
        nsgrid was temporarily removed and replaced with pkdtree due to issues
        relating to its reliability and accuracy (Issues #2919, #2229, #2345,
        #2670, #2930)
+    .. versionchanged:: 1.0.2
+       enabled nsgrid again
     """
-    # TODO: add 'nsgrid': _nsgrid_capped back once fixed
     methods = {'bruteforce': _bruteforce_capped_self,
-               'pkdtree': _pkdtree_capped_self}
+               'pkdtree': _pkdtree_capped_self,
+               'nsgrid': _nsgrid_capped_self,
+    }
 
     if method is not None:
         return methods[method.lower()]
@@ -888,8 +896,7 @@ def _determine_method_self(reference, max_cutoff, min_cutoff=None, box=None,
     if max_cutoff < 0.03*size.min():
         return methods['pkdtree']
     else:
-        # Replaced nsgrid with pkdtree temporarily #2930
-        return methods['pkdtree']
+        return methods['nsgrid']
 
 
 @check_coords('reference', enforce_copy=False, reduce_result_if_single=False)
@@ -1126,9 +1133,9 @@ def _nsgrid_capped_self(reference, max_cutoff, min_cutoff=None, box=None,
             gridsearch = FastNS(max_cutoff, reference, box=box)
             results = gridsearch.self_search()
 
-        pairs = results.get_pairs()[::2, :]
+        pairs = results.get_pairs()
         if return_distances or (min_cutoff is not None):
-            distances = results.get_pair_distances()[::2]
+            distances = results.get_pair_distances()
             if min_cutoff is not None:
                 idx = distances > min_cutoff
                 pairs, distances = pairs[idx], distances[idx]

diff --git a/package/MDAnalysis/lib/include/calc_distances.h b/package/MDAnalysis/lib/include/calc_distances.h
@@ -31,7 +31,7 @@ typedef float coordinate[3];
   #define USED_OPENMP 0
 #endif
 
-static void minimum_image(double* x, float* box, float* inverse_box)
+void minimum_image(double* x, float* box, float* inverse_box)
 {
   int i;
   double s;
@@ -43,7 +43,7 @@ static void minimum_image(double* x, float* box, float* inverse_box)
   }
 }
 
-static void minimum_image_triclinic(double* dx, float* box)
+void minimum_image_triclinic(double* dx, float* box)
 {
    /*
     * Minimum image convention for triclinic systems, modelled after domain.cpp