Expand Gabor Kernel Generation to nD #2739

kne42 · 2017-07-28T20:32:11Z

Description

Expands the Gabor filter to work on images of any dimension.

Checklist

Clean style in the spirit of PEP8
Docstrings for all functions
Gallery example in ./doc/examples (new features only)
Unit tests

[For detailed information on these and other aspects see scikit-image contribution guidelines]

References

Closes Gabor filters are not 3-D compatible #2704.
Yun T, Ling G. 2010. Human Emotion Recognition Using Real 3D Visual Features from Gabor Library. 2010 IEEE International Workshop on Multimedia Signal Processing Saint Malo, France. https://doi.org/10.1109/MMSP.2010.5662073
Wang Y, Chua C. 2005. Face recognition from 2D and 3D images using 3D Gabor filters. School of Electrical and Electronic Engineering, Nanyang Technological University Singapore, Singapore. https://doi.org/10.1016/j.imavis.2005.07.005
Nguyen TM. 2014. N-Dimensional Quasipolar Coordinates - Theory and Application. University of Nevada, Las Vegas Las Vegas, NV. https://digitalscholarship.unlv.edu/thesesdissertations/2125
Zhelezov OI. 2018. One Modification which Increases Performance of N-Dimensional rotation Matrix Generation Algorithm. International Journal of Chemistry, Mathematics, and Physics 2(2): pp. 13-18. https://dx.doi.org/10.22161/ijcmp.2.2.1
ter Haar Romeny BM. 2003. Front-End Vision and Multi-Scale Image Analysis. Computation Imaging and Vision Vol. 27. Springer: Dordrecht, Netherlands: pp. 37-51. https://doi.org/10.1007/978-1-4020-8840-7_3

For reviewers

(Don't remove the checklist below.)

Check that the PR title is short, concise, and will make sense 1 year
later.
Check that new functions are imported in corresponding __init__.py.
Check that new features, API changes, and deprecations are mentioned in
doc/release/release_dev.rst.

pep8speaks · 2017-07-28T20:32:13Z

Hello @kne42! Thanks for updating the PR.

In the file skimage/filters/_gabor.py, following are the PEP8 issues :

Line 181:80: E501 line too long (89 > 79 characters)
Line 182:80: E501 line too long (89 > 79 characters)
Line 183:80: E501 line too long (89 > 79 characters)
Line 184:80: E501 line too long (89 > 79 characters)
Line 185:80: E501 line too long (89 > 79 characters)

In the file skimage/filters/tests/test_gabor.py, following are the PEP8 issues :

Line 59:47: E127 continuation line over-indented for visual indent
Line 60:47: E127 continuation line over-indented for visual indent
Line 61:47: E127 continuation line over-indented for visual indent
Line 62:47: E127 continuation line over-indented for visual indent

Comment last updated on May 30, 2018 at 21:40 Hours UTC

jni · 2018-04-26T00:02:06Z

skimage/filters/_gabor.py

+
+        # handle deprecation
+        message = ('Using deprecated, 2D-only interface to'
+                   'gabor_kernel. This interface will be'


You need spaces at the end of each sub-string, since concatenation within parentheses won't add spaces. =)

jni · 2018-04-26T00:33:03Z

skimage/filters/_gabor.py

+        message = ('Using deprecated, 2D-only interface to'
+                   'gabor_kernel. This interface will be'
+                   'removed in scikit-image 0.16. Use'
+                   'gabor_kernel(frequency, rotation=theta,'


You say rotation=theta here but the signature does not have a rotation keyword. fwiw I think theta as a kwarg can be maintained, silently adding support for multidimensional theta.

jni · 2018-04-26T00:47:25Z

skimage/filters/_gabor.py

-    if sigma_y is None:
-        sigma_y = _sigma_prefactor(bandwidth) / frequency
+    def __new__(cls, frequency, thetas=None, bandwidth=1, sigma=None,
+                sigma_y=None, n_stds=3, offset=None, ndim=2, **kwargs):


I'm not sure that the complexity of a class is necessary. Why did you pick this design instead of extending the existing function?

@jni I chose this design because it encourages reusing the existing gabor kernel. As of right now, the filters.gabor function creates a new gabor kernel every time you use it. This becomes very expensive if I, say, want to apply the same gabor filter to 1000 images.

While there is already a method of eliminating this expense by creating a gabor kernel and applying it to the images yourself, I would posit that most users would not consider this as they would immediately gravitate towards the gabor function, which is not obviously expensive from the documentation when applied many times. Furthermore, if one wanted to reproduce what the gabor function did, they would have to look at the source code and figure out exactly how we were able to apply the filter to an image.

Although I generally prefer functional design to object-oriented design, I think that this choice is justified for the following reasons:

NumPy is already object-oriented. If we're representing the kernel as an np.ndarray anyways, it would be most useful to extend this interface to apply it as well.

This tags along the filter function to the kernel instead of having to use one function to make the kernel and another to apply it (this would be the functional alternative). This is also a lot more intuitive.

@kne42 I think the simplest option is to add to filters.gabor a kernel parameter that is the (bare) NumPy array, and avoid recomputing the kernel if this argument is given.

The NumPy community has moved away from inheriting from NumPy arrays because it raises a whole bunch of problems. An example: suppose someone comes up with a fancy new "Gabor++" method that works by creating a Gabor kernel, modifying in some weird way, then applying it. Any call to np.copy(kernel) will strip away the class and return a bare array. Then you've lost your way of applying the kernel.

The other issue is the "surprise" factor: this is a pattern that does not exist anywhere in skimage, or NumPy/SciPy for that matter. So now users have to wrap their heads around a whole new API for Gabor kernels. Or they run g = filters.gabor_kernel(freq) and then type type(g) and get a strange class that they've never seen.

jni · 2018-04-26T00:53:46Z

@kne42 this is looking nice! Please ping when it's ready for a full review.

Speaking of pinging, did you receive my email? I used the address on your GH profile.

kne42 · 2018-04-30T12:50:01Z

Excellent points; I’ll change it now.

…

On Mon, Apr 30, 2018 at 7:09 AM Juan Nunez-Iglesias < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In skimage/filters/_gabor.py <#2739 (comment)> : > @@ -72,30 +90,134 @@ def gabor_kernel(frequency, theta=0, bandwidth=1, sigma_x=None, sigma_y=None, >>> io.imshow(gk.real) # doctest: +SKIP >>> io.show() # doctest: +SKIP """ - if sigma_x is None: - sigma_x = _sigma_prefactor(bandwidth) / frequency - if sigma_y is None: - sigma_y = _sigma_prefactor(bandwidth) / frequency + def __new__(cls, frequency, thetas=None, bandwidth=1, sigma=None, + sigma_y=None, n_stds=3, offset=None, ndim=2, **kwargs): @kne42 <https://github.com/kne42> I think the simplest option is to add to filters.gabor a kernel parameter that is the (bare) NumPy array, and avoid recomputing the kernel if this argument is given. The NumPy community has moved away from inheriting from NumPy arrays because it raises a whole bunch of problems. An example: suppose someone comes up with a fancy new "Gabor++" method that works by creating a Gabor kernel, modifying in some weird way, then applying it. Any call to np.copy(kernel) will strip away the class and return a bare array. Then you've lost your way of applying the kernel. The other issue is the "surprise" factor: this is a pattern that does not exist anywhere in skimage, or NumPy/SciPy for that matter. So now users have to wrap their heads around a whole new API for Gabor kernels. Or they run g = filters.gabor_kernel(freq) and then type type(g) and get a strange class that they've never seen. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2739 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ab0F02WkGfDaoOq_2ZKSpRvaSAfua252ks5ttvDogaJpZM4OnDPx> .

…rror that used by scipy.ndimage

…ates as a utility

…tes, added tests

…'use_homogeneous_coords'

kne42 · 2018-05-16T14:47:11Z

@jni @emmanuelle This PR is now ready for its penultimate review as I start cleaning up documentation/implementation, working out kinks in the functions, and expanding unit test coverage.

Some points of discussion:

What to do with all of these helper functions. Which, if any, do you think should be made public? If we make them public, should we introduce this refactor in a separate PR for cleanliness or just make the changes in this one?
Deprecation handling & backwards-compatibility have resulted in some less-than-ideal design choices. How do you think we should address this?
Thoughts on general design/clarity?

…into gabor-nD

codecov-io · 2018-05-21T16:26:33Z

Codecov Report

Merging #2739 into master will increase coverage by 0.05%.
The diff coverage is 96.25%.

@@            Coverage Diff            @@
##           master   #2739      +/-   ##
=========================================
+ Coverage   86.05%   86.1%   +0.05%     
=========================================
  Files         338     338              
  Lines       27364   27504     +140     
=========================================
+ Hits        23547   23683     +136     
- Misses       3817    3821       +4

Impacted Files	Coverage Δ
skimage/filters/tests/test_gabor.py	`98.09% <100%> (+2.01%)`	⬆️
skimage/filters/_gabor.py	`94.73% <94.28%> (-5.27%)`	⬇️
skimage/draw/_random_shapes.py	`97.89% <0%> (+2.1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef78faf...abea636. Read the comment docs.

stefanv · 2018-05-28T17:22:23Z

skimage/filters/_gabor.py

+
+    norm = np.linalg.norm(u)
+
+    if not np.isclose(norm, 1):


Why is this check necessary?

stefanv · 2018-05-28T17:23:37Z

skimage/filters/_gabor.py

+
+
+def _compute_projection_matrix(axis, indices=None):
+    """Generates a matrix that projects an axis onto the 0th coordinate axis.


What is the 0th coordinate axis?

stefanv · 2018-05-28T17:24:01Z

skimage/filters/_gabor.py

+
+    References
+    ----------
+    .. [1] Bart M. ter Haar Romeny. Front-End Vision and Multi-Scale Image


This is a collection of articles; could you add the name (and perhaps page number) of the relevant one?

stefanv · 2018-05-28T17:24:42Z

skimage/filters/_gabor.py

+    >>> Y = np.asarray([0.5, 0.5])
+
+    >>> M = _compute_rotation_matrix(X, Y)
+    >>> Z = np.matmul(M, X)


stefanv · 2018-05-28T17:25:54Z

skimage/filters/_gabor.py

+    Parameters
+    ----------
+    image : non-complex array
+        Linear space to map the filter to.


How is the image a linear space here?

stefanv · 2018-05-28T17:28:25Z

skimage/filters/_gabor.py

+        Standard deviation for Gaussian kernel. The standard deviations of the
+        Gaussian filter are given for each axis as a sequence, or as a single
+        number, in which case it is equal for all axes.
+    ndim : int, optional


I'm wondering here: should we allow center and sigma to be scalars? If not, we can derive ndim from center and sigma.

stefanv · 2018-05-28T17:29:09Z

skimage/filters/_gabor.py

    offset : float, optional
        Phase offset of harmonic function in radians.
+    axes : int or sequence of int, optional
+        Ordering of axes that defines the plane of rotation. Ordering not


Not sure what this parameter does?

stefanv · 2018-05-28T17:29:48Z

skimage/filters/_gabor.py

-    if sigma_y is None:
-        sigma_y = _sigma_prefactor(bandwidth) / frequency
+    # handle deprecation
+    message = ('Using deprecated, 2D-only interface to gabor_kernel. '


Is there no way to support the old style calling convention still?

stefanv · 2018-05-28T17:30:32Z

skimage/filters/_gabor.py

+    m = np.asarray(np.meshgrid(*[range(-c, c + 1) for c in spatial_size],
+                               indexing='ij'))
+
+    rotm = np.matmul(m.T, rot.T).T


stefanv · 2018-05-28T17:39:21Z

skimage/filters/_gabor.py

+    R = np.eye(ndim)  # Initial rotation matrix = Identity matrix
+
+    # Loop to create matrices of stages
+    for step in np.round(2 ** np.arange(np.log2(ndim))).astype(int):


Clarify what happens here: why the log2, the 2**, etc.

I suggest linking to

https://en.wikipedia.org/wiki/Rotation_matrix#In_three_dimensions

and pointing out that this function will compute the correct matrices for rotation along individual axes in the same way that is described for 3D in that page.

Add the same links to the general generate_rotation_matrix function

stefanv · 2018-05-28T17:40:44Z

skimage/filters/_gabor.py

+            if n + step >= len(w):
+                break
+
+            r2 = x[w[n]] * x[w[n]] + x[w[n + step]] * x[w[n + step]]


r2 = x[w[n]] ** 2 + x[w[n + step]] ** 2

Ah, but then we take the sqrt, so how about np.hypot?

stefanv · 2018-05-28T17:42:41Z

skimage/filters/_gabor.py

+            r2 = x[w[n]] * x[w[n]] + x[w[n + step]] * x[w[n + step]]
+            if r2 > 0:
+                r = np.sqrt(r2)
+                pcos = x[w[n]] / r  # Calculation of coefficients


Again, x[w[n]] and x[w[n + step]]. Since these indices are re-used, should we name them?

stefanv · 2018-05-28T17:49:14Z

skimage/filters/_gabor.py

+    dst : (N, ) array
+        Vector of desired direction.
+    use_homogeneous_coords : bool, optional
+        If the input vectors should be treated as homoegeneous coordinates.


homogeneous

stefanv · 2018-05-28T17:50:58Z

skimage/filters/_gabor.py

+    r : float
+        Radial coordinate.
+    thetas : (N, ) array
+        Quasipolar angles.


Some explanation of where these angles lie would be helpful (given the "quasi" nature you explained to me).

soupault added the 💪 Work in progress label Jul 28, 2017

kne42 changed the title ~~[WIP][Enhancement] N-Dimensional Gabor Filter~~ ENH: N-Dimensional Gabor Filter Jul 28, 2017

This was referenced Aug 6, 2017

Added function for interpreting and expanding multidimensional parameters. #2747

Closed

WIP: (2/4) n-Dimensional Geometric Transformations #2748

Closed

kne42 changed the title ~~ENH: N-Dimensional Gabor Filter~~ (4/4) N-Dimensional Gabor Filter Aug 6, 2017

kne42 force-pushed the gabor-nD branch 2 times, most recently from c62318b to 5fbfe7a Compare April 19, 2018 19:06

kne42 changed the title ~~(4/4) N-Dimensional Gabor Filter~~ n-Dimensional Gabor Filter Apr 19, 2018

Completed initial refactor of gabor; handled deprecated interface.

e49d4b8

kne42 force-pushed the gabor-nD branch 2 times, most recently from f4dac92 to 923bcb2 Compare April 21, 2018 22:55

kne42 added 2 commits April 21, 2018 19:09

Fixed argument broadcasting typos.

76755c9

Implemented quasipolar coordinate conversion.

9cbe0c5

kne42 force-pushed the gabor-nD branch from 923bcb2 to 9cbe0c5 Compare April 21, 2018 23:10

kne42 added 2 commits April 21, 2018 19:17

Fixed various syntactical errors.

6b0f630

Improved gabor kernel implementation template.

c73f567

kne42 force-pushed the gabor-nD branch from 715c265 to c73f567 Compare April 25, 2018 14:58

kne42 changed the title ~~n-Dimensional Gabor Filter~~ n-Dimensional Gabor Filter (WIP) Apr 25, 2018

jni reviewed Apr 26, 2018

View reviewed changes

Fixed typos as pointed out by jni.

7ae5b03

jni reviewed Apr 26, 2018

View reviewed changes

Make multidimensional theta change silent.

bb9abdb

kne42 added 3 commits April 25, 2018 20:56

Made filters.gabor act as a wrapper around filter.gabor_kernel.apply

ebca9ac

Implemented multivariate gaussian function.

dd824a1

Cleaned up gaussian function a little bit more.

621a6d6

Refactored OO kernel to functional kernel implementation.

3879610

kne42 added 9 commits May 15, 2018 09:25

quasipolar_coordinates: changed documentation of 'leading_axis' to mi…

d3e143a

…rror that used by scipy.ndimage

quasipolar_decomposition: implemented axis ordering

b8463f5

quasipolar_decomposition: added 'Notes' section to documentation

66018e7

compute_rotation_matrix: can now treat vectors as homogeneous coordin…

63eed8f

…ates as a utility

compute_rotation_matrix: fixed implementation of homogeneous coordina…

0d8cfc0

…tes, added tests

compute_rotation_matrix: refactored 'use_homogeneous_coordinates' to …

8ccbd8f

…'use_homogeneous_coords'

gabor: fixed various syntactical errors

f0fcca1

gabor: improved documentation

e70545f

gabor: added references for 3D gabor articles

84316e6

kne42 added 3 commits May 16, 2018 13:52

gabor: properly compute quasipolar sum of m

1e81f92

gabor: fixed various style/doctest errors

32dfd7e

gabor: fixed various PEP8 errors

a3fb878

kne42 changed the title ~~n-Dimensional Gabor Filter (WIP)~~ Expand Gabor Kernel Generation to nD May 16, 2018

Merge branch 'master' of https://github.com/scikit-image/scikit-image …

9bff807

…into gabor-nD

stefanv reviewed May 28, 2018

View reviewed changes

kne42 added 5 commits May 28, 2018 14:21

gabor: clarified documentation; cleaned code

75d752b

gabor: removed unecessary 'ndim' parameters

f9de2dc

gabor: remove Python 2 support

571952e

gabor: added deprecation warning for 'axes'

f39dc3f

added comments that explain MNMRG algorithm

05ccc52

kne42 force-pushed the gabor-nD branch from 30d0e2e to 05ccc52 Compare May 30, 2018 21:40

kne42 mentioned this pull request May 30, 2018

Add nD rotation matrix generation #3125

Open

7 tasks

gabor: greatly improved test suite

abea636

jni mentioned this pull request May 12, 2020

gabor filter and PCA whitening in ndimage scipy/scipy#11583

Closed

ryanlu41 mentioned this pull request Sep 9, 2020

Gabor filters are not 3-D compatible #2704

Open

Base automatically changed from master to main February 18, 2021 18:22

mkcor mentioned this pull request Feb 22, 2021

2021's calendar of community management #5169

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand Gabor Kernel Generation to nD #2739

Expand Gabor Kernel Generation to nD #2739

kne42 commented Jul 28, 2017 •

edited

pep8speaks commented Jul 28, 2017 •

edited

jni Apr 26, 2018

jni Apr 26, 2018

jni Apr 26, 2018

kne42 Apr 27, 2018

jni Apr 30, 2018

jni commented Apr 26, 2018

kne42 commented Apr 30, 2018 via email

kne42 commented May 16, 2018

codecov-io commented May 21, 2018 •

edited

stefanv May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018 •

edited

stefanv May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018 •

edited

stefanv May 28, 2018

jni May 28, 2018

jni May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018

stefanv May 28, 2018



		def _compute_projection_matrix(axis, indices=None):
		"""Generates a matrix that projects an axis onto the 0th coordinate axis.

Expand Gabor Kernel Generation to nD #2739

Are you sure you want to change the base?

Expand Gabor Kernel Generation to nD #2739

Conversation

kne42 commented Jul 28, 2017 • edited

Description

Checklist

References

For reviewers

pep8speaks commented Jul 28, 2017 • edited

Comment last updated on May 30, 2018 at 21:40 Hours UTC

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jni commented Apr 26, 2018

kne42 commented Apr 30, 2018 via email

kne42 commented May 16, 2018

codecov-io commented May 21, 2018 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanv May 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanv May 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kne42 commented Jul 28, 2017 •

edited

pep8speaks commented Jul 28, 2017 •

edited

codecov-io commented May 21, 2018 •

edited

stefanv May 28, 2018 •

edited

stefanv May 28, 2018 •

edited