RTK icon indicating copy to clipboard operation
RTK copied to clipboard

WIP: Add FFTHilbertImageFilter

Open acoussat opened this issue 4 years ago • 8 comments

First attempt at creating a filter to compute the Hilbert transform of an image.

A few questions:

  • there are no tests of this new filter; should I write some?
  • the kernel is a band-limited Hilbert kernel, but there are no ways in the current implementation to choose that band-limit, should that be an option?

Thanks!

acoussat avatar Jul 29 '20 11:07 acoussat

I have implemented all your comments, thanks again!

I have also created a test for this new filter, but it might still require some work. Specifically:

  • I think I correctly disabled zero padding to write the test, but I don't exactly get the result I am supposed to (the one obtained analytically). Am I supposed to manually mirror the signal out of its current bounds? Does RTK already has an option somewhere to do this?
  • I don't really know how to pick good parameters for CheckImageQuality, as of now they are nothing but random choices. Any tip about that?

Thanks!

acoussat avatar Nov 25 '20 15:11 acoussat

Here is a comparison representing the current state of the test. The signal t -> sin(20*pi*t) has the analytic Hilbert transform t -> -cos(20*pi*t). The current implementation kind of matches the analytic inverse, but still seems incorrect. hilbert_compare I'm unsure about how to improve that... There is probably something to do with the way zero padding is performed, but I'm not sure I understand. Any pointers?

acoussat avatar Dec 18 '20 16:12 acoussat

Hi,

Hopefully a helpful pointer (and a request :-)) for a monogenic signal filter:

https://arxiv.org/abs/1703.09199

which is the multi-dimensional analog to the analytic signal from the Hilbert transform. We have an AnalyticSignalImageFilter here:

https://github.com/KitwareMedical/ITKUltrasound/blob/master/include/itkAnalyticSignalImageFilter.h

thewtex avatar Dec 18 '20 22:12 thewtex

Thanks. itkAnalyticSignalImageFilter.hseems to be doing the same thing as the current rtkHilbertImageFilter.h (which was badly named because we followed Matlab). Is this going to be moved to an ITK module at some point?

SimonRit avatar Dec 19 '20 13:12 SimonRit

@SimonRit Yes, we will make ITKUltrasound available as an ITK remote module following @dzenanz 's work to support remote module dependencies in ITK 5.2:

https://github.com/InsightSoftwareConsortium/ITK/commit/fa1082b92953b09c48ae0d65a96ddfbfd33c7c36#diff-e7efda826b6d097e47d7cb2cd6b1cf126084197d7834e7b022048183509c774b

It would be nice to also have ultrasound filters depend on the CUDA filter support in RTK. I am wondering if now is the time to create a CUDA base module, and a module for FFT extensions, e.g. analytic signal, etc. that the ultrasound and RTK modules could share as dependencies. What do you think?

thewtex avatar Dec 21 '20 16:12 thewtex

Sounds good. For CUDA, it might be easy but we should fix #296. For FFT extensions, RTK would certainly benefit from the analytic signal. We also have filters which work line-by-line or slice-by-slice, the mother class is rtk::FFTProjectionsConvolutionImageFilter. They are pretty specific to tomographic reconstruction but they might be useful in other contexts. I'm sure you'll have some changes to suggest on the code to make it better!

SimonRit avatar Dec 22 '20 16:12 SimonRit

I'm not sure I understand the conclusion of this discussion. Do we keep the current implementation of the Hilbert transform? Should I improve it somehow, for instance by including the analytic signal? Should it be moved somewhere, like to a module that contains FFT extensions?

acoussat avatar Jan 25 '21 17:01 acoussat

I understood that I should finish https://github.com/KitwareMedical/ITKUltrasound/pull/123 before proceeding. Then we should take the parts of ITKUltrasound which overlap ones in RTK and put them in a new common module on which both ITKUltrasound and RTK would depend.

dzenanz avatar Jan 25 '21 17:01 dzenanz