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

Add morlet filters #2152

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

siraferradans
Copy link

We just added the option to the Gabor filter of taking out the DC. This is equivalent to the Morlet filter. In the code we uploaded, we just put the option no_DC_offset=True for generating the Morlet filter (False for Gabor). We were wondering if this is a good option or if you prefer to write a stand-alone function called Morlet that directly calls the Gabor function.

We also added a small example that shows the difference between Morlet and Gabor, and a test to validate that the sum of the filter is zero, when the Morlet option is active. By the way, when executing the tests we get an error that doesnt seem to be related to our test:

======================================================================
ERROR: test suite for <module 'skimage.filters.tests.test_gabor' from '../scikit-image/skimage/filters/tests/test_gabor.py'>
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ferradans/anaconda3/lib/python3.5/site-packages/nose/suite.py", line 229, in run
    self.tearDown()
  File "/Users/ferradans/anaconda3/lib/python3.5/site-packages/nose/suite.py", line 352, in tearDown
    self.teardownContext(ancestor)
  File "/Users/ferradans/anaconda3/lib/python3.5/site-packages/nose/suite.py", line 368, in teardownContext
    try_run(context, names)
  File "/Users/ferradans/anaconda3/lib/python3.5/site-packages/nose/util.py", line 453, in try_run
    inspect.getargspec(func)
  File "/Users/ferradans/anaconda3/lib/python3.5/inspect.py", line 1041, in getargspec
    stacklevel=2)
DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() instead

----------------------------------------------------------------------
Ran 7 tests in 0.796s

Please let us know what you think about the code/options.

CC @eickenberg


plt.suptitle('Morlet (different scales and orientations)')

plt.show()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add new line

@ahojnnes
Copy link
Member

@siraferradans Thanks for your contribution. I left you a few comments and one question.

@eickenberg
Copy link

Thanks for the comments!

@siraferradans
Copy link
Author

Hello,
thanks a lot for such a quick and thorough feed back! So, I started addressing the _gaussian_harmonic_kernel() change, and as I was changing the code, it became clear that the simplest option was to set two functions: gabor_kernel and morlet_kernel, and morlet_kernel calls the gabor_kernel for the code that they were sharing before. Is this ok? I also added a small test example in the plot_gabor_vs_morlet that shows the impact on the energy of the kernel a DC shift.

im_filtered = np.abs(ndi.convolve(image, gabor, mode='wrap'))
print('[Gabor] energy:',im_filtered.sum())
im_filtered100 = np.abs(ndi.convolve(image+100, gabor, mode='wrap'))
print('[Gabor] energy (im+100):',im_filtered100.sum())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to plot these also.

And they should probably go before the plt.show

@codecov-io
Copy link

codecov-io commented Jun 15, 2016

Current coverage is 90.53% (diff: 100%)

Merging #2152 into master will decrease coverage by 0.06%

@@             master      #2152   diff @@
==========================================
  Files           304        297     -7   
  Lines         21425      21289   -136   
  Methods           0          0          
  Messages          0          0          
  Branches       1844       1955   +111   
==========================================
- Hits          19411      19274   -137   
- Misses         1661       1665     +4   
+ Partials        353        350     -3   

Powered by Codecov. Last update 1e94bac...3c66fa4

@siraferradans
Copy link
Author

Dear all,
I have an implementation question now: In order to implement the scattering, we need the wavelet transform, with morlet filters. I saw that scikit-image has support for pywt, but morlet filters are not supported now. This leaves the only option of implementing the wavelet transform directly. I thought that a good idea would be to define a list of list for all the filters, that would be saved in the fourier domain. In this context the convolution of the signal with the filter would be implemented as a multiplication in the Fourier domain.
Is this implementation ok? do you have any comments, maybe references where to see a similar code already available in scikit-image?
Thanks a lot for the possible comments and recommendations.

@eickenberg
Copy link

you should probably post this as a separate issue, since it is not related
to this PR.
There has been some discussion about convolution in Fourier domain (
#1618) and about a
reworking of linear filtering in the feature requests (
https://github.com/scikit-image/scikit-image/wiki/Requested-features) which
you should reference in this case to restart where these ideas left off.
[There has also been a wavelet denoising PR (
https://github.com//pull/1833), but as you say, it
isn't clear whether pywavelets is applicable to the proposed scattering
transform]

On Thu, Jun 16, 2016 at 6:16 PM, Sira Ferradans notifications@github.com
wrote:

Dear all,
I have an implementation question now: In order to implement the
scattering, we need the wavelet transform, with morlet filters. I saw that
scikit-image has support for pywt, but morlet filters are not supported
now. This leaves the only option of implementing the wavelet transform
directly. I thought that a good idea would be to define a list of list for
all the filters, that would be saved in the fourier domain. In this context
the convolution of the signal with the filter would be implemented as a
multiplication in the Fourier domain.
Is this implementation ok? do you have any comments, maybe references
where to see a similar code already available in scikit-image?
Thanks a lot for the possible comments and recommendations.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2152 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABPwC43vXc3Xn6YCw7Y7FupRQwZAvT1Wks5qMXb6gaJpZM4I2Ndl
.

@grlee77
Copy link
Contributor

grlee77 commented Jun 17, 2016

@siraferradans, @eickenberg
just FYI, there is an open PR (PyWavelets/pywt#172) to add the Morlet and several other continuous wavelets to pywt. The implementation is via convolution with filter banks. I can't give an estimate of when it would be completed, but any feedback you might provide there would be welcome.

@siraferradans
Copy link
Author

@grlee77 thanks for the link. I completely agree that those wavelets are interesting for both pywt and sci-kit image. It will definetely be necessary to review this code when pywt gets integrated in scikit image.

@eickenberg
Copy link

@grlee77 Thanks for the heads-up! This looks like an interesting addition to pywavelets. Once it is merged, it may call for a refactoring of the whole gabor module.

@siraferradans siraferradans changed the title [WIP] add morlet filters [MRG] add morlet filters Jun 20, 2016
@sciunto
Copy link
Member

sciunto commented Aug 27, 2016

@siraferradans pywt is now integrated to scikit-image ;)

@soupault soupault changed the title [MRG] add morlet filters Add morlet filters Oct 25, 2016
Base automatically changed from master to main February 18, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants