Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discretization of disk footprints (also ellipse, ball) #5470

Open
grlee77 opened this issue Jul 11, 2021 · 3 comments
Open

Discretization of disk footprints (also ellipse, ball) #5470

grlee77 opened this issue Jul 11, 2021 · 3 comments

Comments

@grlee77
Copy link
Contributor

grlee77 commented Jul 11, 2021

Description

When constructing footprints (structuring elements) corresponding to a disk, the following code is currently used:

L = np.arange(-radius, radius + 1)
X, Y = np.meshgrid(L, L)
return np.array((X ** 2 + Y ** 2) <= radius ** 2, dtype=dtype)

A second possibility is the following:

return np.array((X ** 2 + Y ** 2) <= (radius + 0.5) ** 2, dtype=dtype)

where the radius has been extended by 0.5 voxels. Practially the former means the edge of the disk falls at the voxel center a distance radius away, while for the later, the disk extends to the outer edge of that same voxel. A concrete illustration of this is given for radius = 3 (vs. 3.5) below:

morphology_discrete_illustration_sm

Below is an illustration where the top row are the currently returned footprints for skimage.morphology.disk and the bottom row are the footprints with the proposed modification.

morphology_disks

I don't know that the existing behavior is technically a bug, but it is more a matter of what convention is chosen. To me, the lower row look visually more disk-like (although radius=1 is too coarse to resemble a disk in either case).

If this change is made for disk, the same convention should be used for ball and ellipse as well.

@grlee77
Copy link
Contributor Author

grlee77 commented Jul 19, 2021

Additional considerations:
1.) calling footprints.ellipse(radius, radius) resembles the "proposed" disks more than the "current" ones, although they are not identical for all choices of radius.
2.) The "current" implementation is consistent with the disks produced by Matlab (there are existing tests that verify this)
3.) OpenCV has a "MORPH_ELLIPSE" shape, but apparently not a disk one. If I make ellipses of the corresponding size they actually end up slightly asymmetric, resembling "current" along one axis, but "proposed" along the other! (seems undesirable)

We could potentially allow both conventions by adding a keyword-only strict_radius parameter with default value of True corresponding to the current behavior and a value of False corresponding to the proposed shape here.

morph_ocv

@grlee77
Copy link
Contributor Author

grlee77 commented Jul 19, 2021

ImageMagick deals with this by allowing specification of a floating point radius which for example matches "current" when the radius is a whole number or "proposed" if the radius is a half pixel number. Size is always (floor(radius) * 2 + 1, floor(radius) * 2 + 1)
see: https://legacy.imagemagick.org/Usage/morphology/#disk

@rfezzani
Copy link
Member

rfezzani commented Jul 20, 2021

Hi @grlee77, and thank you for raising this. I agree, your proposed implementation looks visually more disk-like!
I like the idea of strict_radius argument 👍

@scikit-image scikit-image locked and limited conversation to collaborators Oct 18, 2021
@grlee77 grlee77 reopened this Feb 20, 2022
@scikit-image scikit-image unlocked this conversation Feb 20, 2022
@lagru lagru added the 🐛 Bug label Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants