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.

Sign up for GitHub

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

Jump to bottom

torchaudio resampling bug in notebook and updates #6

Open
carolineechen opened this issue Jun 24, 2021 · 2 comments · May be fixed by #7
Open

torchaudio resampling bug in notebook and updates #6

carolineechen opened this issue Jun 24, 2021 · 2 comments · May be fixed by #7

Comments

@carolineechen
Copy link

Hi @jonashaag, thanks for compiling and updating this notebook on resampling libraries! However, I noticed there was a small bug in the new torchaudio examples, and also wanted to update you on some new resampling capabilities from our latest release.

I noticed that there was a bug in the notebook with the torchaudio_new_resample arguments that led to it producing strange plots for both the upsample and downsample examples. Removing [None] and [0] from torchaudio_new_resample(torch.from_numpy(sig[None]), P, Q).numpy()[0] should resolve this issue, and both resampling methods in torchaudio should produce the same results.

In the latest torchaudio release (v0.9.0), we made several improvements to resampling. We improved the speed performance of transforms.Resample, deprecated kaldi.resample_waveform in favor of functional.resample, added kaiser window support, among others. If you're interested, you can track the updates here, and we have also created a tutorial demonstrating the properties of our improved resampling function.

What are your thoughts on the following?

  • fixing the torchaudio_new_resample bug
  • using the latest version of torchaudio (0.9.0) (Note that for the latest version,transforms.Resample should take in an additional dtype=torch.float64 argument in your examples)
  • plotting with a higher quality set of parameters (ex/ using lowpass_filter_width=64 or resampling_method="kaiser_window")

Let me know if you would like to work on this, or if you'd like me to send in a PR, I'd be happy to do so as well!

cc @mthrok
@jonashaag
Copy link
Owner

Thanks for reporting this! I’m happy to merge the changes but I don’t have the bandwidth to work on it myself so a PR would be very welcome.

My suggestion would be to plot the new default settings and if you want one additional higher quality setting. The notebook is already very crowded so I’m against adding more even more examples.

Also I guess it makes sense to add a hint that older PyTorch versions will perform significantly worse.

If you have the bandwidth I’m also happy to merge any other changes to improve the code, eg. splitting the code into separate scripts that generate plots, making a proper benchmark script, GPU benchmarks, etc.

@carolineechen carolineechen linked a pull request Jun 30, 2021 that will close this issue
Open
@carolineechen
Copy link
Author

Hi @jonashaag, just wanted to follow up on this -- I have created a PR (#7) to resolve this. Let me know if you have any comments!

Repository owner deleted a comment from ARRNAV26 Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update torchaudio resampling carolineechen/audio-resampling-in-python
2 participants
@jonashaag @carolineechen