Temporarily disable nsgrid (Issue MDAnalysis#2930)

package/MDAnalysis/lib/distances.py

@@ -336,7 +336,7 @@ def capped_distance(reference, configuration, max_cutoff, min_cutoff=None,
     An automatic guessing of the optimal method to calculate the distances is
     included in the function. An optional keyword for the method is also
     provided. Users can enforce a particular method with this functionality.
-    Currently brute force, grid search, and periodic KDtree methods are
+    Currently brute force and periodic KDtree methods are
     implemented.
 
     Parameters
@@ -354,7 +354,7 @@ def capped_distance(reference, configuration, max_cutoff, min_cutoff=None,
         triclinic and must be provided in the same format as returned by
         :attr:`MDAnalysis.coordinates.base.Timestep.dimensions`:\n
         ``[lx, ly, lz, alpha, beta, gamma]``.
-    method : {'bruteforce', 'nsgrid', 'pkdtree'}, optional
+    method : {'bruteforce', 'pkdtree'}, optional
         Keyword to override the automatic guessing of the employed search
         method.
     return_distances : bool, optional
@@ -387,14 +387,18 @@ def capped_distance(reference, configuration, max_cutoff, min_cutoff=None,
 
     Note
     -----
-    Currently supports brute force, grid-based, and periodic KDtree search
+    Currently supports brute force, and periodic KDtree search
     methods.
 
     See Also
     --------
     distance_array
     MDAnalysis.lib.pkdtree.PeriodicKDTree.search
-    MDAnalysis.lib.nsgrid.FastNS.search
+
+
+    .. versionchanged:: 1.0.1
+       nsgrid was temporarily removed and replaced with pkdtree due to issues
+       surround its reliability (Issues #2919, #2229, #2345, #2670, #2930)
     """
     if box is not None:
         box = np.asarray(box, dtype=np.float32)
@@ -430,18 +434,23 @@ def _determine_method(reference, configuration, max_cutoff, min_cutoff=None,
         triclinic and must be provided in the same format as returned by
         :attr:`MDAnalysis.coordinates.base.Timestep.dimensions`:\n
         ``[lx, ly, lz, alpha, beta, gamma]``.
-    method : {'bruteforce', 'nsgrid', 'pkdtree'}, optional
+    method : {'bruteforce', 'pkdtree'}, optional
         Keyword to override the automatic guessing of the employed search
         method.
 
     Returns
     -------
     function : callable
         The function implementing the guessed (or deliberatly chosen) method.
+
+
+    .. versionchanged:: 1.0.1
+       nsgrid was temporarily removed and replaced with pkdtree due to issues
+       surround its reliability (Issues #2919, #2229, #2345, #2670, #2930)
     """
+    # TODO: add 'nsgrid': _nsgrid_capped back once fixed
     methods = {'bruteforce': _bruteforce_capped,
-               'pkdtree': _pkdtree_capped,
-               'nsgrid': _nsgrid_capped}
+               'pkdtree': _pkdtree_capped}
 
     if method is not None:
         return methods[method.lower()]
@@ -451,7 +460,8 @@ 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
-        return methods['nsgrid']
+        # Temporarily replace nsgrid with pkdtree Issue #2930
+        return methods['pkdtree']
     else:
         if box is None:
             min_dim = np.array([reference.min(axis=0),
@@ -467,7 +477,8 @@ def _determine_method(reference, configuration, max_cutoff, min_cutoff=None,
         if np.any(max_cutoff > 0.3*size):
             return methods['bruteforce']
         else:
-            return methods['nsgrid']
+            # Temporarily replace nsgrid with pkdtree Issue #2930
+            return methods['pkdtree']
 
 
 @check_coords('reference', 'configuration', enforce_copy=False,
@@ -741,7 +752,7 @@ def self_capped_distance(reference, max_cutoff, min_cutoff=None, box=None,
     An automatic guessing of the optimal method to calculate the distances is
     included in the function. An optional keyword for the method is also
     provided. Users can enforce a particular method with this functionality.
-    Currently brute force, grid search, and periodic KDtree methods are
+    Currently brute force, and periodic KDtree methods are
     implemented.
 
     Parameters
@@ -757,7 +768,7 @@ def self_capped_distance(reference, max_cutoff, min_cutoff=None, box=None,
         triclinic and must be provided in the same format as returned by
         :attr:`MDAnalysis.coordinates.base.Timestep.dimensions`:\n
         ``[lx, ly, lz, alpha, beta, gamma]``.
-    method : {'bruteforce', 'nsgrid', 'pkdtree'}, optional
+    method : {'bruteforce', 'pkdtree'}, optional
         Keyword to override the automatic guessing of the employed search
         method.
     return_distances : bool, optional
@@ -789,17 +800,20 @@ def self_capped_distance(reference, max_cutoff, min_cutoff=None, box=None,
 
     Note
     -----
-    Currently supports brute force, grid-based, and periodic KDtree search
+    Currently supports brute force, and periodic KDtree search
     methods.
 
     See Also
     --------
     self_distance_array
     MDAnalysis.lib.pkdtree.PeriodicKDTree.search
-    MDAnalysis.lib.nsgrid.FastNS.self_search
+
 
     .. versionchanged:: 0.20.0
        Added `return_distances` keyword.
+    .. versionchanged:: 1.0.1
+       nsgrid was temporarily removed and replaced with pkdtree due to issues
+       surround its reliability (Issues #2919, #2229, #2345, #2670, #2930)
     """
     if box is not None:
         box = np.asarray(box, dtype=np.float32)
@@ -833,18 +847,23 @@ def _determine_method_self(reference, max_cutoff, min_cutoff=None, box=None,
         triclinic and must be provided in the same format as returned by
         :attr:`MDAnalysis.coordinates.base.Timestep.dimensions`:\n
         ``[lx, ly, lz, alpha, beta, gamma]``.
-    method : {'bruteforce', 'nsgrid', 'pkdtree'}, optional
+    method : {'bruteforce', 'pkdtree'}, optional
         Keyword to override the automatic guessing of the employed search
         method.
 
     Returns
     -------
     function : callable
         The function implementing the guessed (or deliberatly chosen) method.
+
+
+    .. versionchanged:: 1.0.1
+       nsgrid was temporarily removed and replaced with pkdtree due to issues
+       surround its reliability (Issues #2919, #2229, #2345, #2670, #2930)
     """
+    # TODO: add 'nsgrid': _nsgrid_capped back once fixed
     methods = {'bruteforce': _bruteforce_capped_self,
-               'pkdtree': _pkdtree_capped_self,
-               'nsgrid': _nsgrid_capped_self}
+               'pkdtree': _pkdtree_capped_self}
 
     if method is not None:
         return methods[method.lower()]
@@ -865,7 +884,8 @@ def _determine_method_self(reference, max_cutoff, min_cutoff=None, box=None,
     if max_cutoff < 0.03*size.min():
         return methods['pkdtree']
     else:
-        return methods['nsgrid']
+        # Replaced nsgrid with pkdtree temporarily #2930
+        return methods['pkdtree']
 
 
 @check_coords('reference', enforce_copy=False, reduce_result_if_single=False)

package/MDAnalysis/lib/nsgrid.pyx

@@ -70,6 +70,15 @@ not reflect in the results.
 
 .. versionadded:: 0.19.0
 
+Important Note
+--------------
+
+Several issues have been raised about the nsgrid code (#2919, #2229, #2345,
+#2670, #2930). In summary, this code currently yield unreliable/inaccurate
+results. Until fixed, it should not be used. We have left this module in
+place as we prepare an fix for these errors. In the meantime, use of this
+library is at your own perils.
+
 Classes
 -------
 """

testsuite/MDAnalysisTests/lib/test_distances.py

@@ -96,7 +96,7 @@ def test_capped_distance_noresults():
            np.array([[0.1, 0.1, 0.1],
                      [0.2, 0.1, 0.1]], dtype=np.float32))
 
-method_1 = ('bruteforce', 'pkdtree', 'nsgrid')
+method_1 = ('bruteforce', 'pkdtree') # add nsgrid back
 
 min_cutoff_1 = (None, 0.1)
 
@@ -220,7 +220,7 @@ def test_self_capped_distance(npoints, box, method, min_cutoff, ret_dist):
                          [(1, 0.02, '_bruteforce_capped_self'),
                           (1, 0.2, '_bruteforce_capped_self'),
                           (600, 0.02, '_pkdtree_capped_self'),
-                          (600, 0.2, '_nsgrid_capped_self')])
+                          (600, 0.2, '_pkdtree_capped_self')]) # tmp switch to pkdtree
 def test_method_selfselection(box, npoints, cutoff, meth):
     np.random.seed(90003)
     points = (np.random.uniform(low=0, high=1.0,
@@ -235,9 +235,9 @@ def test_method_selfselection(box, npoints, cutoff, meth):
 @pytest.mark.parametrize('npoints,cutoff,meth',
                          [(1, 0.02, '_bruteforce_capped'),
                           (1, 0.2, '_bruteforce_capped'),
-                          (200, 0.02, '_nsgrid_capped'),
+                          (200, 0.02, '_pkdtree_capped'), # tmp switch to pkdtree
                           (200, 0.35, '_bruteforce_capped'),
-                          (10000, 0.35, '_nsgrid_capped')])
+                          (10000, 0.35, '_pkdtree_capped')]) # tmp switch to pkdtree
 def test_method_selection(box, npoints, cutoff, meth):
     np.random.seed(90003)
     points = (np.random.uniform(low=0, high=1.0,
@@ -991,7 +991,7 @@ def test_input_unchanged_self_distance_array(self, coords, box, backend):
         assert_equal(crd, ref)
 
     @pytest.mark.parametrize('box', boxes)
-    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", "nsgrid", None])
+    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", None]) # add nsgrid back
     def test_input_unchanged_capped_distance(self, coords, box, met):
         crds = coords[:2]
         refs = [crd.copy() for crd in crds]
@@ -1000,7 +1000,7 @@ def test_input_unchanged_capped_distance(self, coords, box, met):
         assert_equal(crds, refs)
 
     @pytest.mark.parametrize('box', boxes)
-    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", "nsgrid", None])
+    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", None]) # add nsgrid back
     def test_input_unchanged_self_capped_distance(self, coords, box, met):
         crd = coords[0]
         ref = crd.copy()
@@ -1101,7 +1101,7 @@ def test_empty_input_self_distance_array(self, empty_coord, box, backend):
     @pytest.mark.parametrize('box', boxes)
     @pytest.mark.parametrize('min_cut', [min_cut, None])
     @pytest.mark.parametrize('ret_dist', [False, True])
-    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", "nsgrid", None])
+    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", None]) # add nsgrid back
     def test_empty_input_capped_distance(self, empty_coord, min_cut, box, met,
                                          ret_dist):
         res = distances.capped_distance(empty_coord, empty_coord,
@@ -1117,7 +1117,7 @@ def test_empty_input_capped_distance(self, empty_coord, min_cut, box, met,
     @pytest.mark.parametrize('box', boxes)
     @pytest.mark.parametrize('min_cut', [min_cut, None])
     @pytest.mark.parametrize('ret_dist', [False, True])
-    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", "nsgrid", None])
+    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", None]) # add nsgrid back
     def test_empty_input_self_capped_distance(self, empty_coord, min_cut, box,
                                               met, ret_dist):
         res = distances.self_capped_distance(empty_coord,
@@ -1234,7 +1234,7 @@ def test_output_type_self_distance_array(self, incoords, box, backend):
     @pytest.mark.parametrize('min_cut', [min_cut, None])
     @pytest.mark.parametrize('ret_dist', [False, True])
     @pytest.mark.parametrize('incoords', list(comb(coords, 2)))
-    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", "nsgrid", None])
+    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", None]) # add nsgrid back
     def test_output_type_capped_distance(self, incoords, min_cut, box, met,
                                          ret_dist):
         res = distances.capped_distance(*incoords, max_cutoff=self.max_cut,
@@ -1257,7 +1257,7 @@ def test_output_type_capped_distance(self, incoords, min_cut, box, met,
     @pytest.mark.parametrize('min_cut', [min_cut, None])
     @pytest.mark.parametrize('ret_dist', [False, True])
     @pytest.mark.parametrize('incoords', coords)
-    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", "nsgrid", None])
+    @pytest.mark.parametrize('met', ["bruteforce", "pkdtree", None]) # add nsgrid back
     def test_output_type_self_capped_distance(self, incoords, min_cut, box,
                                               met, ret_dist):
         res = distances.self_capped_distance(incoords,