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

ENH: Add trough_threshold parameter to feature.sat.pitch #117

Merged
merged 8 commits into from Jan 31, 2024

Conversation

ralphpeterson
Copy link
Contributor

I was trying to compute the fundamental frequency of some ultrasonic vocalizations and noticed that the pitch function (vocalpy.feature.sat) returned wonky results. It turns out that the librosa yin function referenced in pitch has an additional argument, trough_threshold that when changed helps produce better f0 estimates (see example below with black line showing f0 estimate from librosa.yin). Not sure exactly the underlying reason, but nonetheless we should have access to this argument in vocalpy.

In this PR I simply added trough_threshold as an argument in the pitch function.

Untitled

added trough_threshold argument to yin algorithm
type edit
@ralphpeterson ralphpeterson changed the title Add trough threshold to yin Add trough_threshold argument to librosa.yin Jan 29, 2024
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d58d43f) 79.75% compared to head (a07aa8e) 79.75%.
Report is 2 commits behind head on main.

❗ Current head a07aa8e differs from pull request most recent head 875f669. Consider uploading reports for the commit 875f669 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #117   +/-   ##
=======================================
  Coverage   79.75%   79.75%           
=======================================
  Files          29       29           
  Lines         968      968           
=======================================
  Hits          772      772           
  Misses        196      196           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NickleDave NickleDave changed the title Add trough_threshold argument to librosa.yin ENH: Add trough_threshold parameter to feature.sat.pitch Jan 31, 2024
@NickleDave
Copy link
Contributor

Awesome, thanks so much @ralphpeterson.

We definitely needed to add this. The visual is quite helpful -- maybe we should add something like that to docs at some point, e.g. a plot in the feature.sat.pitch docstring.

Overall this looks great, I just made a couple tweaks, like adding the same parameter to feature.sat.similarity_features.

Merging in! Much appreciated

@NickleDave NickleDave merged commit 8624fc4 into vocalpy:main Jan 31, 2024
9 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants