Skip to content

Commit

Permalink
Standardize on rng over seed and fix miscellaneous deprecation wa…
Browse files Browse the repository at this point in the history
…rnings (#621)

This PR fixes the following warnings observed when running the test suite:

```
src/cucim/core/operations/morphology/tests/test_distance_transform.py: 90 warnings
  /pyenv/versions/3.9.18/lib/python3.9/site-packages/cucim/core/operations/morphology/_distance_transform.py:160: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)
    scalar_sampling = float(unique_sampling)
```

```
 src/cucim/skimage/feature/tests/test_corner.py::test_custom_eigvals_kernels_vs_linalg_eigvalsh[float64-shape2]
  /__w/cucim/cucim/python/cucim/src/cucim/skimage/feature/tests/test_corner.py:345: FutureWarning: use_gaussian_derivatives currently defaults to False, but will change to True in a future version. Please specify this argument explicitly to maintain the current behavior
    H = hessian_matrix(img)
```

```
 src/cucim/skimage/morphology/tests/test_binary.py: 148 warnings
  /__w/cucim/cucim/python/cucim/src/cucim/skimage/morphology/tests/test_binary.py:68: FutureWarning: `seed` is a deprecated argument name for `binary_blobs`. It will be removed in version 0.23. Please use `rng` instead.
    img = cp.asarray(data.binary_blobs(32, n_dim=ndim, seed=1))
```

Authors:
  - Gregory Lee (https://github.com/grlee77)

Approvers:
  - Gigon Bae (https://github.com/gigony)
  - https://github.com/jakirkham

URL: #621
  • Loading branch information
grlee77 authored Nov 4, 2023
1 parent 5e25641 commit f26381a
Show file tree
Hide file tree
Showing 15 changed files with 240 additions and 120 deletions.
8 changes: 4 additions & 4 deletions benchmarks/skimage/cucim_metrics_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ def __init__(
run_cpu=run_cpu,
)

def _generate_labels(self, dtype, seed=5):
def _generate_labels(self, dtype, rng=5):
ndim = len(self.shape)
blobs_kwargs = dict(
blob_size_fraction=0.05, volume_fraction=0.35, seed=seed
blob_size_fraction=0.05, volume_fraction=0.35, rng=rng
)
# binary blobs only creates square outputs
labels = measure.label(
Expand All @@ -67,8 +67,8 @@ def _generate_labels(self, dtype, seed=5):
return labels.astype(dtype, copy=False)

def set_args(self, dtype):
labels1_d = self._generate_labels(dtype, seed=5)
labels2_d = self._generate_labels(dtype, seed=3)
labels1_d = self._generate_labels(dtype, rng=5)
labels2_d = self._generate_labels(dtype, rng=3)
labels1 = cp.asnumpy(labels1_d)
labels2 = cp.asnumpy(labels2_d)
self.args_cpu = (labels1, labels2)
Expand Down
4 changes: 2 additions & 2 deletions benchmarks/skimage/cucim_segmentation_bench.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def __init__(
def _generate_labels(self, dtype):
ndim = len(self.shape)
blobs_kwargs = dict(
blob_size_fraction=0.05, volume_fraction=0.35, seed=5
blob_size_fraction=0.05, volume_fraction=0.35, rng=5
)
# binary blobs only creates square outputs
labels = measure.label(
Expand Down Expand Up @@ -107,7 +107,7 @@ def set_args(self, dtype):
n_dim = len(self.shape)
data = cucim.skimage.img_as_float(
cucim.skimage.data.binary_blobs(
length=max(self.shape), n_dim=n_dim, seed=1
length=max(self.shape), n_dim=n_dim, rng=1
)
)
data = data[tuple(slice(s) for s in self.shape)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import numpy as np
import cupy as cp

from ._pba_2d import _pba_2d
from ._pba_3d import _pba_3d
Expand Down Expand Up @@ -153,7 +153,7 @@ def distance_transform_edt(
"""
scalar_sampling = None
if sampling is not None:
unique_sampling = np.unique(np.atleast_1d(sampling))
unique_sampling = cp.unique(cp.atleast_1d(sampling))
if len(unique_sampling) == 1:
# In the isotropic case, can use the kernels without sample scaling
# and just adjust the final distance accordingly.
Expand Down
3 changes: 1 addition & 2 deletions python/cucim/src/cucim/skimage/_shared/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,7 @@ def check_random_state(seed):
if isinstance(seed, cp.random.RandomState):
return seed
raise ValueError(
"%r cannot be used to seed a numpy.random.RandomState"
" instance" % seed
f"{seed} cannot be used to seed a cupy.random.RandomState instance"
)


Expand Down
29 changes: 15 additions & 14 deletions python/cucim/src/cucim/skimage/data/_binary_blobs.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import cupy as cp

from .._shared.filters import gaussian
from .._shared.utils import deprecate_kwarg


@deprecate_kwarg(
{"seed": "rng"}, deprecated_version="23.12.00", removed_version="24.12.00"
)
def binary_blobs(
length=512, blob_size_fraction=0.1, n_dim=2, volume_fraction=0.5, seed=None
length=512, blob_size_fraction=0.1, n_dim=2, volume_fraction=0.5, rng=None
):
"""
Generate synthetic binary image with several rounded blob-like objects.
Expand All @@ -19,12 +25,10 @@ def binary_blobs(
volume_fraction : float, default 0.5
Fraction of image pixels covered by the blobs (where the output is 1).
Should be in [0, 1].
seed : {None, int, `cupy.random.Generator`}, optional
If `seed` is None the `cupy.random.Generator` singleton is used.
If `seed` is an int, a new ``Generator`` instance is used,
seeded with `seed`.
If `seed` is already a ``Generator`` instance then that instance is
used.
rng : {`cupy.random.Generator`, int}, optional
Pseudo-random number generator.
By default, a PCG64 generator is used (see :func:`cupy.random.default_rng`).
If `rng` is an int, it is used to seed the generator.
Returns
-------
Expand All @@ -34,7 +38,7 @@ def binary_blobs(
Notes
-----
Warning: CuPy does not give identical randomly generated numbers as NumPy,
so using a specific seed here will not give an identical pattern to the
so using a specific `rng` here will not give an identical pattern to the
scikit-image implementation.
The behavior for a given random seed may also change across CuPy major
Expand All @@ -45,19 +49,16 @@ def binary_blobs(
--------
>>> from cucim.skimage import data
>>> # tiny size (5, 5)
>>> blobs = data.binary_blobs(length=5, blob_size_fraction=0.2, seed=1)
>>> blobs = data.binary_blobs(length=5, blob_size_fraction=0.2)
>>> # larger size
>>> blobs = data.binary_blobs(length=256, blob_size_fraction=0.1)
>>> # Finer structures
>>> blobs = data.binary_blobs(length=256, blob_size_fraction=0.05)
>>> # Blobs cover a smaller volume fraction of the image
>>> blobs = data.binary_blobs(length=256, volume_fraction=0.3)
"""
# filters is quite an expensive import since it imports all of scipy.signal
# We lazy import here
from .._shared.filters import gaussian
""" # noqa: E501

rs = cp.random.default_rng(seed)
rs = cp.random.default_rng(rng)
shape = tuple([length] * n_dim)
mask = cp.zeros(shape)
n_pts = max(int(1.0 / blob_size_fraction) ** n_dim, 1)
Expand Down
6 changes: 6 additions & 0 deletions python/cucim/src/cucim/skimage/data/tests/test_data.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import cupy as cp
import pytest
from numpy.testing import assert_almost_equal

from cucim.skimage import data
Expand All @@ -15,3 +16,8 @@ def test_binary_blobs():
length=32, volume_fraction=0.25, n_dim=3
)
assert not cp.all(blobs == other_realization)


def test_binary_blobs_futurewarning():
with pytest.warns(FutureWarning):
data.binary_blobs(length=128, seed=5)
6 changes: 3 additions & 3 deletions python/cucim/src/cucim/skimage/feature/tests/test_corner.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ def _reference_eigvals_computation(S_elems):
def test_custom_eigvals_kernels_vs_linalg_eigvalsh(shape, dtype):
rng = cp.random.default_rng(seed=5)
img = rng.integers(0, 256, shape)
H = hessian_matrix(img)
H = hessian_matrix(img, use_gaussian_derivatives=False)
H = tuple(h.astype(dtype, copy=False) for h in H)
evs1 = _reference_eigvals_computation(H)
evs2 = hessian_matrix_eigvals(H)
Expand Down Expand Up @@ -492,8 +492,8 @@ def test_corner_foerstner_dtype(dtype):
def test_noisy_square_image():
im = cp.zeros((50, 50)).astype(float)
im[:25, :25] = 1.0
np.random.seed(seed=1234) # result is specific to this NumPy seed
im = im + cp.asarray(np.random.uniform(size=im.shape)) * 0.2
rng = np.random.default_rng(1234) # result is specific to this NumPy seed
im = im + cp.asarray(rng.uniform(size=im.shape)) * 0.2

# # Moravec
# results = peak_local_max(corner_moravec(im),
Expand Down
4 changes: 2 additions & 2 deletions python/cucim/src/cucim/skimage/filters/tests/test_edges.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ def test_vertical_mask_line(grad_func):
],
)
def test_3d_edge_filters(func, max_edge):
blobs = binary_blobs(length=128, n_dim=3, seed=5)
blobs = binary_blobs(length=128, n_dim=3, rng=5)
edges = func(blobs)
center = max_edge.shape[0] // 2
if center == 2:
Expand All @@ -663,7 +663,7 @@ def test_3d_edge_filters(func, max_edge):
],
)
def test_3d_edge_filters_single_axis(func, max_edge):
blobs = binary_blobs(length=128, n_dim=3, seed=5)
blobs = binary_blobs(length=128, n_dim=3, rng=5)
edges0 = func(blobs, axis=0)
center = max_edge.shape[0] // 2
if center == 2:
Expand Down
35 changes: 25 additions & 10 deletions python/cucim/src/cucim/skimage/morphology/_skeletonize.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,14 @@ def _get_tiebreaker(n, seed):


@deprecate_kwarg(
{"random_state": "seed"},
{"random_state": "rng"},
deprecated_version="23.08",
removed_version="24.06",
)
def medial_axis(image, mask=None, return_distance=False, *, seed=None):
@deprecate_kwarg(
{"seed": "rng"}, deprecated_version="23.12", removed_version="24.12"
)
def medial_axis(image, mask=None, return_distance=False, *, rng=None):
"""Compute the medial axis transform of a binary image.
Parameters
Expand All @@ -187,14 +190,17 @@ def medial_axis(image, mask=None, return_distance=False, *, seed=None):
value in `mask` are used for computing the medial axis.
return_distance : bool, optional
If true, the distance transform is returned as well as the skeleton.
seed : {None, int, `numpy.random.Generator`}, optional
If `seed` is None, the `numpy.random.Generator` singleton is used.
If `seed` is an int, a new ``Generator`` instance is used, seeded with
`seed`.
If `seed` is already a ``Generator`` instance, then that instance is
used.
rng : {`numpy.random.Generator`, int}, optional
Pseudo-random number generator.
By default, a PCG64 generator is used
(see :func:`numpy.random.default_rng`).
If `rng` is an int, it is used to seed the generator.
The PRNG determines the order in which pixels are processed for
tiebreaking.
.. versionadded:: 0.19
Note: Due to a missing `permute` method on CuPy's random Generator
class, only a `numpy.random.Generator` is currently supported.
Returns
-------
Expand Down Expand Up @@ -304,7 +310,16 @@ def medial_axis(image, mask=None, return_distance=False, *, seed=None):
# We use a random # for tiebreaking. Assign each pixel in the image a
# predictable, random # so that masking doesn't affect arbitrary choices
# of skeletons
tiebreaker = _get_tiebreaker(n=distance.size, seed=seed)

if rng is None or isinstance(rng, int):
tiebreaker = _get_tiebreaker(n=distance.size, seed=rng)
elif isinstance(rng, np.random.Generator):
generator = np.random.default_rng(rng)
tiebreaker = cp.asarray(generator.permutation(np.arange(distance.size)))
else:
raise ValueError(
f"{type(rng)} class not yet supported for use in " "`medial_axis`."
)
order = cp.lexsort(
cp.stack((tiebreaker, corner_score[masked_image], distance), axis=0)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def _get_decomp_test_data(function, ndim=2):
img = cp.zeros((17,) * ndim, dtype=cp.uint8)
img[(8,) * ndim] = 1
else:
img = cp.asarray(data.binary_blobs(32, n_dim=ndim, seed=1))
img = cp.asarray(data.binary_blobs(32, n_dim=ndim, rng=1))
return img


Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import cupy as cp
import numpy as np
import pytest
from cupy.testing import assert_array_equal
from skimage import data
Expand Down Expand Up @@ -93,7 +94,7 @@ def test_00_01_zeros_masked(self):
result = medial_axis(cp.zeros((10, 10), bool), cp.zeros((10, 10), bool))
assert not cp.any(result)

def test_vertical_line(self):
def _test_vertical_line(self, **kwargs):
"""Test a thick vertical line, issue #3861"""
img = cp.zeros((9, 9))
img[:, 2] = 1
Expand All @@ -103,9 +104,35 @@ def test_vertical_line(self):
expected = cp.full(img.shape, False)
expected[:, 3] = True

result = medial_axis(img)
result = medial_axis(img, **kwargs)
assert_array_equal(result, expected)

def test_vertical_line(self):
"""Test a thick vertical line, issue #3861"""
self._test_vertical_line()

def test_rng_numpy(self):
# NumPy Generator allowed
self._test_vertical_line(rng=np.random.default_rng())

def test_rng_cupy(self):
# CuPy Generator not currently supported
with pytest.raises(ValueError):
self._test_vertical_line(rng=cp.random.default_rng())

def test_rng_int(self):
self._test_vertical_line(rng=15)

def test_vertical_line_seed(self):
"""seed was deprecated (now use rng)"""
with pytest.warns(FutureWarning):
self._test_vertical_line(seed=15)

def test_vertical_line_random_state(self):
"""random_state was deprecated (now use rng)"""
with pytest.warns(FutureWarning):
self._test_vertical_line(random_state=15)

def test_01_01_rectangle(self):
"""Test skeletonize on a rectangle"""
image = cp.zeros((9, 15), bool)
Expand Down
29 changes: 15 additions & 14 deletions python/cucim/src/cucim/skimage/restoration/deconvolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,15 @@ def wiener(image, psf, balance, reg=None, is_real=True, clip=True):


@deprecate_kwarg(
{"random_state": "seed"},
{"random_state": "rng"},
removed_version="23.08.00",
deprecated_version="24.06.00",
)
@deprecate_kwarg(
{"seed": "rng"},
removed_version="23.08.00",
deprecated_version="24.12.00",
)
def unsupervised_wiener(
image,
psf,
Expand All @@ -160,7 +165,7 @@ def unsupervised_wiener(
is_real=True,
clip=True,
*,
seed=None,
rng=None,
):
"""Unsupervised Wiener-Hunt deconvolution.
Expand All @@ -186,12 +191,11 @@ def unsupervised_wiener(
clip : boolean, optional
True by default. If true, pixel values of the result above 1 or
under -1 are thresholded for skimage pipeline compatibility.
seed : {None, int, `numpy.random.Generator`}, optional
If `seed` is None, the `numpy.random.Generator` singleton is used.
If `seed` is an int, a new ``Generator`` instance is used, seeded with
`seed`.
If `seed` is already a ``Generator`` instance, then that instance is
used.
rng : {`cupy.random.Generator`, int}, optional
Pseudo-random number generator.
By default, a PCG64 generator is used
(see :func:`cupy.random.default_rng`).
If `rng` is an int, it is used to seed the generator.
Returns
-------
Expand Down Expand Up @@ -233,7 +237,8 @@ def unsupervised_wiener(
>>> img = color.rgb2gray(cp.array(data.astronaut()))
>>> psf = cp.ones((5, 5)) / 25
>>> img = ndi.uniform_filter(img, size=psf.shape)
>>> img += 0.1 * img.std() * cp.random.standard_normal(img.shape)
>>> rng = cp.random.default_rng()
>>> img += 0.1 * img.std() * rng.standard_normal(img.shape)
>>> deconvolved_img = restoration.unsupervised_wiener(img, psf)
Notes
Expand Down Expand Up @@ -321,11 +326,7 @@ def unsupervised_wiener(
else:
data_spectrum = uft.ufft2(image)

try:
rng = cp.random.default_rng(seed)
except AttributeError:
# older CuPy without default_rng
rng = cp.random.RandomState(seed)
rng = cp.random.default_rng(rng)

# Gibbs sampling
for iteration in range(params["max_num_iter"]):
Expand Down
Loading

0 comments on commit f26381a

Please sign in to comment.