search

maxrmorrison / torchcrepe

Pytorch implementation of the CREPE pitch tracker
MIT License
407 stars 63 forks source link

fix wrong argument order to `core.postprocess` #20

Closed tandav closed 1 year ago

tandav commented 1 year ago

Current code in master branch passes return_periodicity as 5th argument to postprocess in the predict function. The 5th argument of postprocess is return_harmonicity, but return_periodicity is passed instead.

https://github.com/maxrmorrison/torchcrepe/blob/9aecc86f5f3ef908bd75656368639686480800e0/torchcrepe/core.py#L127-L131 https://github.com/maxrmorrison/torchcrepe/blob/9aecc86f5f3ef908bd75656368639686480800e0/torchcrepe/core.py#L566-L571

This leads to DeprecationWarning even when return_harmonicity is not used.

DeprecationWarning: The torchcrepe return_harmonicity argument is deprecated and will be removed in a future release. Please use return_periodicity. Rationale: if network confidence measured harmonics, the value would be low for non-harmonic, periodic sounds (e.g., sine waves). But this is not observed.                                                                                                                       
    warnings.warn(message, DeprecationWarning)

This PR fixes this bug.

maxrmorrison commented 1 year ago

Looks good. Thanks for catching that.

tandav commented 1 year ago

Can you please update the PyPI torchcrepe package as well?

maxrmorrison commented 1 year ago

Updated. You might also take a look at this repo with a similar API and better accuracy + speed

tandav commented 1 year ago

Thank you! Yes, I've seen penn, tried it and accuracy is great!