Fix wrap bug in ndfilters convolve and correlate (#243)

* Bug fixing ndfilters.convolve and ndfilter.correlate

* Added tests

* Fixed a syntax error

* Update dask_image/ndfilters/_conv.py

Co-authored-by: Genevieve Buckley <30920819+GenevieveBuckley@users.noreply.github.com>

* Update dask_image/ndfilters/_conv.py

Co-authored-by: Genevieve Buckley <30920819+GenevieveBuckley@users.noreply.github.com>

Co-authored-by: Genevieve Buckley <30920819+GenevieveBuckley@users.noreply.github.com>

Why do we set `mode=constant` here? "The reason I did it is because I assume that it is (barely) faster than doing reflect because padding with constant values is easier than padding reflectively." -  anlavandier
Gen also assumes that it is unlikely to cause edge artifacts between chunks, because the user can adjust the map_overlap depth to the point where the scipy chunk boundary modes don't really affect the final Dask result.

dask_image/ndfilters/_conv.py

@@ -13,17 +13,17 @@
 
 
 @_utils._update_wrapper(scipy.ndimage.filters.convolve)
-def convolve(image,
-             weights,
-             mode='reflect',
-             cval=0.0,
-             origin=0):
+def convolve(image, weights, mode="reflect", cval=0.0, origin=0):
     check_arraytypes_compatible(image, weights)
 
     origin = _utils._get_origin(weights.shape, origin)
     depth = _utils._get_depth(weights.shape, origin)
     depth, boundary = _utils._get_depth_boundary(image.ndim, depth, "none")
 
+    if mode == "wrap":  # Fixes https://github.com/dask/dask-image/issues/242
+        boundary = "periodic"
+        mode = "constant"
+
     result = image.map_overlap(
         dispatch_convolve(image),
         depth=depth,
@@ -33,24 +33,24 @@ def convolve(image,
         weights=weights,
         mode=mode,
         cval=cval,
-        origin=origin
+        origin=origin,
     )
 
     return result
 
 
 @_utils._update_wrapper(scipy.ndimage.filters.correlate)
-def correlate(image,
-              weights,
-              mode='reflect',
-              cval=0.0,
-              origin=0):
+def correlate(image, weights, mode="reflect", cval=0.0, origin=0):
     check_arraytypes_compatible(image, weights)
 
     origin = _utils._get_origin(weights.shape, origin)
     depth = _utils._get_depth(weights.shape, origin)
     depth, boundary = _utils._get_depth_boundary(image.ndim, depth, "none")
 
+    if mode == "wrap":  # Fixes https://github.com/dask/dask-image/issues/242
+        boundary = "periodic"
+        mode = "constant"
+
     result = image.map_overlap(
         dispatch_correlate(image),
         depth=depth,
@@ -60,7 +60,7 @@ def correlate(image,
         weights=weights,
         mode=mode,
         cval=cval,
-        origin=origin
+        origin=origin,
     )
 
     return result

tests/test_dask_image/test_ndfilters/test__conv.py

@@ -154,3 +154,35 @@ def test_convolutions_compare(sp_func,
             d, weights, origin=origin
         )
     )
+@pytest.mark.parametrize(
+    "sp_func, da_func",
+    [
+        (scipy.ndimage.filters.convolve, dask_image.ndfilters.convolve),
+        (scipy.ndimage.filters.correlate, dask_image.ndfilters.correlate),
+    ]
+)
+@pytest.mark.parametrize(
+    "weights",
+    [
+     np.ones((1,5)),
+     np.ones((5,1)),
+    ]
+)
+@pytest.mark.parametrize(
+    "mode",
+    ["reflect","wrap","nearest","constant","mirror"])
+def test_convolutions_modes(sp_func,
+                            da_func,
+                            weights,
+                            mode):
+    a = np.arange(140).reshape(10,14)
+    d = da.from_array(a,chunks =(5, 7))
+
+    da.utils.assert_eq(
+        sp_func(
+            a, weights, mode = mode
+        ),
+        da_func(
+            d, weights, mode = mode
+        )
+    )