search

keunwoochoi / torchaudio-contrib

A test bed for updates and new features | pytorch/audio
169 stars 22 forks source link

HTK and Slaney #24

Closed VinodS7 closed 5 years ago

VinodS7 commented 5 years ago

For reproducibility it would be useful to have these two mel filters. These are the two types of filters I have seen the most so correct me if there are other more important types of mel filters.

keunwoochoi commented 5 years ago

Agreed. Implementation wouldn't be a big issue, so would be the API design. PR would be welcome!

VinodS7 commented 5 years ago

Will work on it. I am going to reimplement how librosa generates the filters in their backend. However, it is done in python and not in C. Also, I will write some tests that compare my implementation to librosa's but do we have a system in place for writing and evaluating tests?

hagenw commented 5 years ago

If you want to use the same test framework as PyTorch, uinittest would be the way to go. I would personally prefer pytest which is great for doing parametrized tests, see also https://github.com/pytorch/pytorch/issues/11578.

To compare the actual outcome of the two implementation, something like numpy.array_equal or numpy.testing.assert_almost_equal might be of interest.

keunwoochoi commented 5 years ago

@VinodS7 Welcome and thanks :)

in python and not in C.

Would it make a big difference in computation time?

keunwoochoi commented 5 years ago

@hagenw Thanks for letting us know. Probably we should start with unittest and see how it goes there. @faroit @f0k Any thought?

f0k commented 5 years ago

Probably we should start with unittest and see how it goes there. @faroit @f0k Any thought?

If there is no policy forbidding us to do so, I'd also prefer pytest.

These are the two types of filters I have seen the most so correct me if there are other more important types of mel filters.

I think we should make it easy to reproduce most research papers, so (a) we should include these two filterbanks by default and (b) the API should make it easy to add your own.

keunwoochoi commented 5 years ago

Ok let's go with pytest.

keunwoochoi commented 5 years ago

@VinodS7 Hi, we'd love to hear from you if there's been any progress! :)

ksanjeevan commented 5 years ago

Should be easy to add Slaney following librosa https://librosa.github.io/librosa/_modules/librosa/core/time_frequency.html#hz_to_mel? Would just have to add them as options to MelFilterbank!

VinodS7 commented 5 years ago

@keunwoochoi Hey. Sorry got caught up is some paper deadlines. I'll have it done by end of next week!

keunwoochoi commented 5 years ago

@VinodS7 Perfect, no problem, no rush. @ksanjeevan is working on Melspectrogram and I'm expecting to merge some update on it soon. It'll become obvious on which part of the code you should be work on. Thanks! :D