Merge pull request #1096 from glotzerlab/fix/voronoi-segfault

Compute Class NeighborList Lifetime

ChangeLog.md

@@ -9,6 +9,9 @@ and this project adheres to
 ### Added
 * Filter neighborlists with `freud.locality.FilterSANN` and `freud.locality.FilterRAD`.
 
+### Fixed
+* Neighborlists generated by certain compute objects now exist after the compute object is garbage collected.
+
 ## v2.12.1 -- 2022-12-05
 
 ### Added

doc/source/reference/credits.rst

@@ -335,6 +335,7 @@ Tommy Waltmann
 * Normalize n(r) in ``RDF`` class by number of query points.
 * Contributed code, design, documentation, and testing for ``freud.locality.FilterSANN`` class.
 * Contributed code, design, documentation, and testing for ``freud.locality.FilterRAD`` class.
+* Fixed segfault in neighborlists owned by compute objects.
 
 Maya Martirossyan
 

freud/environment.pyx

@@ -341,7 +341,9 @@ cdef class LocalDescriptors(_PairCompute):
     def nlist(self):
         """:class:`freud.locality.NeighborList`: The neighbor list from the
         last compute."""
-        return freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist = freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist._compute = self
+        return nlist
 
     @_Compute._computed_property
     def sph(self):
@@ -1013,7 +1015,9 @@ cdef class AngularSeparationNeighbor(_PairCompute):
     def nlist(self):
         """:class:`freud.locality.NeighborList`: The neighbor list from the
         last compute."""
-        return freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist = freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist._compute = self
+        return nlist
 
 
 cdef class AngularSeparationGlobal(_Compute):
@@ -1183,7 +1187,9 @@ cdef class LocalBondProjection(_PairCompute):
     def nlist(self):
         """:class:`freud.locality.NeighborList`: The neighbor list from the
         last compute."""
-        return freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist = freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist._compute = self
+        return nlist
 
     @_Compute._computed_property
     def projections(self):

freud/locality.pxd

@@ -40,6 +40,7 @@ cdef class NeighborQuery:
 cdef class NeighborList:
     cdef freud._locality.NeighborList * thisptr
     cdef char _managed
+    cdef freud.util._Compute _compute
 
     cdef freud._locality.NeighborList * get_ptr(self)
     cdef void copy_c(self, NeighborList other)

freud/locality.pyx

@@ -607,6 +607,7 @@ cdef class NeighborList:
         # Cython won't assign NULL without cast
         self.thisptr = <freud._locality.NeighborList *> NULL if _null \
             else new freud._locality.NeighborList()
+        self._compute = None
 
     def __dealloc__(self):
         if self._managed:
@@ -780,6 +781,10 @@ cdef NeighborList _nlist_from_cnlist(freud._locality.NeighborList *c_nlist):
     any compute method that requires a :class:`~.NeighborList` (i.e. cannot do
     with just a :class:`~.NeighborQuery`) should also expose the internally
     computed :class:`~.NeighborList` using this method.
+
+    Args:
+        c_nlist (freud._locality.NeighborList *):
+            C++ neighborlist object.
     """
     cdef NeighborList result
     result = NeighborList()
@@ -1291,6 +1296,7 @@ cdef class Voronoi(_Compute):
             :class:`~.locality.NeighborList`: Neighbor list.
         """
         self._nlist = _nlist_from_cnlist(self.thisptr.getNeighborList().get())
+        self._nlist._compute = self
         return self._nlist
 
     def __repr__(self):
@@ -1402,12 +1408,16 @@ cdef class Filter(_PairCompute):
     @_Compute._computed_property
     def filtered_nlist(self):
         """:class:`.NeighborList`: The filtered neighbor list."""
-        return _nlist_from_cnlist(self._filterptr.getFilteredNlist().get())
+        nlist = _nlist_from_cnlist(self._filterptr.getFilteredNlist().get())
+        nlist._compute = self
+        return nlist
 
     @_Compute._computed_property
     def unfiltered_nlist(self):
         """:class:`.NeighborList`: The unfiltered neighbor list."""
-        return _nlist_from_cnlist(self._filterptr.getUnfilteredNlist().get())
+        nlist = _nlist_from_cnlist(self._filterptr.getUnfilteredNlist().get())
+        nlist._compute = self
+        return nlist
 
 
 cdef class FilterSANN(Filter):

freud/order.pyx

@@ -924,7 +924,9 @@ cdef class SolidLiquid(_PairCompute):
     def nlist(self):
         """:class:`freud.locality.NeighborList`: Neighbor list of solid-like
         bonds."""
-        return freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist = freud.locality._nlist_from_cnlist(self.thisptr.getNList())
+        nlist._compute = self
+        return nlist
 
     @_Compute._computed_property
     def num_connections(self):

tests/conftest.py

@@ -0,0 +1,17 @@
+# Copyright (c) 2010-2023 The Regents of the University of Michigan
+# This file is from the freud project, released under the BSD 3-Clause License.
+
+import freud
+
+
+def nlist_lifetime_check(get_nlist_func):
+    """Ensure nlist exists past the lifetime of the compute that created it."""
+    L = 10
+    N = 100
+    sys = freud.data.make_random_system(L, N)
+
+    nlist = get_nlist_func(sys)
+
+    assert nlist.point_indices is not None
+    assert nlist.query_point_indices is not None
+    assert nlist.neighbor_counts is not None

tests/test_environment_AngularSeparation.py

@@ -1,6 +1,7 @@
 # Copyright (c) 2010-2023 The Regents of the University of Michigan
 # This file is from the freud project, released under the BSD 3-Clause License.
 
+import conftest
 import numpy as np
 import numpy.testing as npt
 import pytest
@@ -51,6 +52,14 @@ def test_compute(self):
             for j in [0, 1]:
                 npt.assert_allclose(ang.angles[i][j], np.pi / 16, atol=1e-6)
 
+    def test_nlist_lifetime(self):
+        def _get_nlist(sys):
+            asn = freud.environment.AngularSeparationNeighbor()
+            asn.compute(sys, orientations=np.zeros((100, 4)), neighbors=dict(r_max=2.0))
+            return asn.nlist
+
+        conftest.nlist_lifetime_check(_get_nlist)
+
     def test_repr(self):
         ang = freud.environment.AngularSeparationGlobal()
         assert str(ang) == str(eval(repr(ang)))

tests/test_environment_LocalBondProjection.py

@@ -1,6 +1,7 @@
 # Copyright (c) 2010-2023 The Regents of the University of Michigan
 # This file is from the freud project, released under the BSD 3-Clause License.
 
+import conftest
 import numpy as np
 import numpy.testing as npt
 import pytest
@@ -33,6 +34,19 @@ def test_nlist(self):
 
         npt.assert_array_equal(nlist[:], ang.nlist[:])
 
+    def test_nlist_lifetime(self):
+        def _get_nlist(sys):
+            lbp = freud.environment.LocalBondProjection()
+            lbp.compute(
+                sys,
+                np.zeros((100, 4)),
+                proj_vecs=np.zeros((100, 3)),
+                neighbors=dict(r_max=2),
+            )
+            return lbp.nlist
+
+        conftest.nlist_lifetime_check(_get_nlist)
+
     def test_attribute_access(self):
         boxlen = 10
         N = 100

tests/test_environment_LocalDescriptors.py

@@ -3,6 +3,7 @@
 
 from functools import lru_cache
 
+import conftest
 import numpy as np
 import numpy.testing as npt
 import pytest
@@ -124,6 +125,16 @@ def test_global(self):
 
         assert sphs.shape[0] == N * num_neighbors
 
+    def test_nlist_lifetime(self):
+        """Ensure the nlist lives past the lifetime of the LocalDescriptors object."""
+
+        def _get_nlist(system):
+            ld = freud.environment.LocalDescriptors(l_max=3)
+            ld.compute(system, neighbors=dict(r_max=2))
+            return ld.nlist
+
+        conftest.nlist_lifetime_check(_get_nlist)
+
     def test_particle_local(self):
         N = 1000
         num_neighbors = 4

tests/test_locality_Filter.py

@@ -3,6 +3,7 @@
 
 from abc import abstractmethod
 
+import conftest
 import numpy as np
 import numpy.testing as npt
 import pytest
@@ -118,6 +119,15 @@ def test_random_system(self, terminate_after_blocked):
         npt.assert_allclose(nlist_1.point_indices, nlist_2.point_indices)
         npt.assert_allclose(nlist_1.query_point_indices, nlist_2.query_point_indices)
 
+    @pytest.mark.parametrize("nlist_property", ["filtered_nlist", "unfiltered_nlist"])
+    def test_nlist_lifetime(self, nlist_property):
+        def _get_nlist(sys):
+            filt = self.get_filter_object()
+            filt.compute(sys)
+            return getattr(filt, nlist_property)
+
+        conftest.nlist_lifetime_check(_get_nlist)
+
 
 class TestRAD(FilterTest):
     """Tests specific to the RAD filtering method."""

tests/test_locality_Voronoi.py

@@ -1,6 +1,7 @@
 # Copyright (c) 2010-2023 The Regents of the University of Michigan
 # This file is from the freud project, released under the BSD 3-Clause License.
 
+import conftest
 import matplotlib
 import matplotlib.pyplot as plt
 import numpy as np
@@ -255,6 +256,16 @@ def test_voronoi_tess_3d(self):
         )
         npt.assert_allclose(wrapped_distances, vor.nlist.distances)
 
+    def test_voronoi_neighbors_lifetime(self):
+        """Ensure the voronoi nlist lives past the lifetime of the voronoi object."""
+
+        def _get_voronoi_nlist(system):
+            voro = freud.locality.Voronoi()
+            voro.compute(system)
+            return voro.nlist
+
+        conftest.nlist_lifetime_check(_get_voronoi_nlist)
+
     @pytest.mark.parametrize(
         "func, neighbors",
         [

tests/test_order_SolidLiquid.py

@@ -1,6 +1,7 @@
 # Copyright (c) 2010-2023 The Regents of the University of Michigan
 # This file is from the freud project, released under the BSD 3-Clause License.
 
+import conftest
 import matplotlib
 import numpy.testing as npt
 import pytest
@@ -73,6 +74,14 @@ def test_multiple_compute(self):
             assert comp_default.cluster_sizes[0] == len(positions)
             npt.assert_array_equal(comp_default.num_connections, 12)
 
+    def test_nlist_lifetime(self):
+        def _get_nlist(sys):
+            sl = freud.order.SolidLiquid(2, 0.5, 0.2)
+            sl.compute(sys, neighbors=dict(r_max=2))
+            return sl.nlist
+
+        conftest.nlist_lifetime_check(_get_nlist)
+
     def test_attribute_access(self):
         box, positions = freud.data.UnitCell.fcc().generate_system(4, scale=2)
         sph_l = 6