Unittests fixes #885
Merged
Unittests fixes #885
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tests were designed for a digital dynamic of [-1 + 1 / 32767, 1] They were modified to pass for a dynamic range of [-1, 1 - 1 / 32767] Add a comment to explain the current status of the Vorbis test
This also modifies DynamicComplexity to use the global SilenceThreshold instead of having a hardcoded value
This way it gets removed even if the test fails
testSpectrumSizeSmallerThanNumberOfBands was failing because of an incorrect implementation. The new way of computing the bands correctly assignates the central frequencies: 57.26, 163.98, 469.57, 1344.60, 3850.23 and 11024.99 Hz in that particular case. This means that only the last band contains one spectral bin. However, this was making the test fail, as it was designed to pass only if no spectral bins were contained inside any band. The test was modified to pass on the current implementation.
In this case the correct method is assertEqualMatrix
testRegression is renamed to testValidResults The new testRegression checks different parameter configurations.
The audio source was changed to a shorter monophonic signal. The expected values are now stored in a .npy so they are lighter and faster to load. Although the output was compared to the vamp implementation and seen to be very close the expected values were obtained from the algorithm itself. This way the allowed error could be reduced so the assertion will be triggered with any small variation from the current values.
This test compares our pitch estimation with the Yin implementation of the Vamp pYin plugging https://code.soundsoftware.ac.uk/projects/pyin
This test compares our YinFFT pitch estimation with the Aubio implementation
Given that every time the algorithm is reset it clears its buffers, Compute/Reset/Compute tests do not make sense for this algorithm
As now we perform normalization inside IFFT by default, it does not have to be added as an extra parameter in the OverlapAdd algorithm for synthesis.
Also the IFFTC does not have to be reversed now thanks to the changes in 449d9b0
Also includes some error fixes in the parameters
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Repairs most of the tests failing or with errors, some of them already reported in #316