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

Remove an invalid resample comparison unit test #1426

Merged
merged 1 commit into from
Apr 5, 2021

Conversation

carolineechen
Copy link
Contributor

@carolineechen carolineechen commented Apr 5, 2021

unit test comparing librosa and torchaudio's numerical results for resampling was not correct -- resampling with a larger range of input data can result in numerical differences of greater than atol=1e-02 in the unit test, which is not enough to claim compatibility

Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks.

nit: I think the test itself is correctly written. It's just the threshold is not tight enough to claim the compatibility.

@carolineechen carolineechen changed the title Remove an incorrect resample comparison unit test Remove an invalid resample comparison unit test Apr 5, 2021
@carolineechen carolineechen merged commit f0ab42c into pytorch:master Apr 5, 2021
mthrok added a commit to mthrok/audio that referenced this pull request Apr 6, 2021
Similar to pytorch#1426 `test_InverseMelScale` in `librosa_compatibility_test` is not
ensuring the comaptibility to librosa. Having this test can give a wrong statement
about the librosa numerical compatibility about the function.
mthrok added a commit to mthrok/audio that referenced this pull request Apr 7, 2021
Similar to pytorch#1426 `test_InverseMelScale` in `librosa_compatibility_test` is not
ensuring the comaptibility to librosa. Having this test can give a wrong statement
about the librosa numerical compatibility about the function.
mthrok added a commit that referenced this pull request Apr 7, 2021
* Remove an invalid InverseMel comparison unit test

Similar to #1426 `test_InverseMelScale` in `librosa_compatibility_test` is not
ensuring the comaptibility to librosa. Having this test can give a wrong statement
about the librosa numerical compatibility about the function.

* Add test for InverseMelScale

The new test compares the result of inverse mel scale against the reference spectrogram, so no need to use librosa.
This test serves as more like an insurance that the change to the implementation of InverseMelScale only improves the result, not the other way.
carolineechen pushed a commit to carolineechen/audio that referenced this pull request Apr 30, 2021
carolineechen pushed a commit to carolineechen/audio that referenced this pull request Apr 30, 2021
* Remove an invalid InverseMel comparison unit test

Similar to pytorch#1426 `test_InverseMelScale` in `librosa_compatibility_test` is not
ensuring the comaptibility to librosa. Having this test can give a wrong statement
about the librosa numerical compatibility about the function.

* Add test for InverseMelScale

The new test compares the result of inverse mel scale against the reference spectrogram, so no need to use librosa.
This test serves as more like an insurance that the change to the implementation of InverseMelScale only improves the result, not the other way.
mthrok pushed a commit to mthrok/audio that referenced this pull request Dec 13, 2022
* Rename ddp_pipeline_tutorial.py to ddp_pipeline.py

This will make this file non-executable and the code will still show up in the file.

* Update build.sh

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

Successfully merging this pull request may close these issues.

None yet

3 participants