Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

htk=true for mel frequencies #27

Closed
justinsalamon opened this issue Feb 20, 2018 · 8 comments
Closed

htk=true for mel frequencies #27

justinsalamon opened this issue Feb 20, 2018 · 8 comments

Comments

@justinsalamon
Copy link

We noticed the current implenetation of the mel_frequencies function (based on Librosa) doesn't include the htk=True option, which is handy when training CNNs because then the frequency scale is fully logarithmic which, in principle, makes more sense for frequency invariant convolutional filters.

What was the motivation for removing this? Any chance it can be added?

@keunwoochoi
Copy link
Owner

There's only one reason; to make it simpler as an initial step. PR would be welcomed :)

@keunwoochoi
Copy link
Owner

It will be added -- actually I'll directly use librosa's mel_frequencies function instead of relying on copied codes.

@justinsalamon
Copy link
Author

Has this been added? Can't see it in the code?

@keunwoochoi
Copy link
Owner

Oh, not yet on the master, it's on dev branch only. https://github.com/keunwoochoi/kapre/blob/unittest/kapre/backend.py, sorry to confuse you!

@justinsalamon
Copy link
Author

justinsalamon commented Mar 1, 2018

Yeah in which case you might want to keep the issue open until the update is merged into master.

Or - you can close issues via commit messages, e.g. "this commit closes #27" - this way when the issue is closed there's a documented link to the fix

@keunwoochoi keunwoochoi reopened this Mar 1, 2018
@keunwoochoi
Copy link
Owner

via commit messages

Oh, didn't know that, interesting!

@justinsalamon
Copy link
Author

More info: https://help.github.com/articles/closing-issues-using-keywords/
Example of what this looks like from my scaper repo: justinsalamon/scaper#23

@keunwoochoi
Copy link
Owner

Finally, with ver>=0.1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants