asteroid-team / asteroid-filterbanks

Asteroid's filterbanks :rocket:
https://asteroid-team.github.io/
MIT License
80 stars 20 forks source link

Question on hilbert transform #13

Open gzhu06 opened 3 years ago

gzhu06 commented 3 years ago

Thanks for this project, very impressive contribution! I have a question on the hibert transform from the code asteroid-filterbanks/asteroid_filterbanks/analytic_free_fb.py :

    ft_f = rfft(self._filters, 1, normalized=True)
    hft_f = conj(ft_f)
    hft_f = irfft(hft_f, 1, normalized=True, signal_sizes=(self.kernel_size,))
    return torch.cat([self._filters, hft_f], dim=0)`

As far as I know, the hilbert transform is performed like this:

  1. Take the real part of the signal;
  2. Rotating the phase of the signal by 90°
  3. Analytical signal = real + i*(rotated signal).

From the code, it looks like using conj to perform rotation operation, is this correct?

mpariente commented 3 years ago

You're absolutely right, thanks a lot! It's a clear oversight I made when adding support to torch 1.8!

For torch under 1.8, it will work as expected though as what I called it conj (in a very stupid way), does the rotation

    def conj(filt):
        return torch.stack([filt[:, :, :, 1], -filt[:, :, :, 0]], dim=-1)

Do you want to submit a fix for torch version > 1.8 with the correct rotation? That would be awesome!

Thanks again for spoting it!

gzhu06 commented 3 years ago

Yes, I'd like to. I'll submit a fix some time this or next week.

Just want to make sure that to fix this: in 1.7, pytorch uses additional dimension to store real and image parts, while in 1.8 it's already a tensor in complex datatype.

So it's simply:

hft_f = -1j * ft_f

Right? Also I think conj could be misleading.

mpariente commented 3 years ago

Yes, I'd like to. I'll submit a fix some time this or next week.

Thanks ! :tada:

Yes this is correct : hft_f = -1j * ft_f (-90° phase shift).

And the conj name should also be changed, you're right