tests for TimeStretch - Githubissues

ksanjeevan commented 5 years ago

Added tests for TimeStretch and some renaming.

A few things:

~Should TimeStretch take in both (batch, channel, num_freq, time, complex=2) as well as with no batch? I tried doing it but index_select returns a copy of the tensor and I couldn't think of a way to do tensor[:,:,slice] where the dimension slice acts on can be set in a clean way.~ Addressed here
Are we using time for the spectrogram time dimension name?
In order to compare our phase_vocoder with the results from librosa.core.phase_vocoder the same precision is needed for some operations, since the defaults cause minor differences and the algorithm does a big cumulative sum, that makes these differences pile up and the results differ significantly. I just changed the default dtype for that test and all matches well.

keunwoochoi commented 5 years ago

Great, thanks! Merging now.

ksanjeevan commented 5 years ago

@keunwoochoi - quick thing, since we went with the overriding_rate option, should we allow for fixed_rate to be None for the augmentation use case? And then throw an error if both are None.

keunwoochoi commented 5 years ago

@ksanjeevan Sounds good. With a nice docstring :)

keunwoochoi commented 5 years ago

@ksanjeevan I think we can keep working on this branch for that patch. Would you?

ksanjeevan commented 5 years ago

Oops I pushed it directly yesterday since it was small..

keunwoochoi commented 5 years ago

Oh cool. I'm deleting this branch.

keunwoochoi / torchaudio-contrib

tests for TimeStretch #65