asteroid-team / asteroid-filterbanks

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

Matching torch.stft and torch.istft #2

Closed mpariente closed 3 years ago

mpariente commented 3 years ago

Those are just first steps but it seems it won't be too hard.

Until now:

More work needs to be done to support w**2 OLA correction after the ISTFT (highpri), center and normalize (lowpri).

onesided will always supposed to be True and we won't support the onesided argument.

Would solve #1

mpariente commented 3 years ago

Cc @Baldwin-disso and @faroit

mpariente commented 3 years ago

The ISTFT looks in a good shape as well. I need to write more informal tests with different windows to make sure it works, but given the C++ code, I'm pretty sure it'll be fine.

mpariente commented 3 years ago

torch.isttf raises RuntimeError for most windows, so we need to use center=True to avoid boundary effects on the w**2 OLA. The forward works with center=True apparently (to be check with other strides etc.. but looks in a good shape)

jonashaag commented 3 years ago

FYI there is a ton of tests here: https://github.com/pytorch/pytorch/blob/master/test/test_spectral_ops.py probably overkill to duplicate all of them but maybe for inspiration! :)

mpariente commented 3 years ago

Thanks for that, it will be a good source of inspiration indeed.

mpariente commented 3 years ago

So the ISTFT also works with center=True, when we don't have the length argument.. And now comes a new problem, we should be able to pass kwargs to the hooks in order to pass the length. But how to organize that?

mpariente commented 3 years ago

Finally passing! Only works for kernel_size == n_filters. I won't spend more time on this.

faroit commented 3 years ago

@mpariente thanks will test that soon