Introduce new unit test files for pitch salience #1121
Conversation
Merge from Master back to my Fork
…66Merge branch 'master' into pitchsalience
|
@dbogdanov, almost ready for review. Early feedback required to help know what to do with subsequent variants of PitchContours (PitchContourMonoMelody, etc) |
|
In draft status, but comments welcome. |
|
TODO: update copyright to 2021 |
|
@dbogdanov , some small updates on Unit Tests: Removed resets. Increased precision to 8 (9cccfc9 ) |
|
In commit "32f1a77a04d0c3b147892af5de8ce046bfa2d4f7" I added the artificial music generator and signal generator for testing. |
|
@dbogdanov Music Generator replaced with raw scale ladder. Only 1 TODO remaining concerning doubts on low confidence output. |
| mags = [0.5, 0.5, 0.5, 0.5] # length 4 | ||
| self.assertRaises(EssentiaException, lambda: PitchSalienceFunction()(freqs, mags)) | ||
|
|
||
| def testARealCase(self): |
There was a problem hiding this comment.
The salience of a given frequency is computed as the sum of the weighted energies found at integer multiples (harmonics) of that frequency.
We are missing a test checking this. For example,
- Give a single peak as an input and validate how all subharmonics are activated in the salience function.
- Create a simple test with only a few input peaks.
The weights of harmonics are defined by parameter harmonicWeight.
- We can test the default 0.8 value.
- Test a simpler value of 1 and 0.
The values 0 and 1 aren't currently allowed for harmonicWeight, but we can enable those as they may be useful in some practical use cases. They are also useful for running tests. We can run simple tests proposed in this comment for both 0 and 1 values, apart from the default 0.8 value. Furthermore, the documentation already mentioned the case of =1 for no decay even though is not enabled.
Note that to make harmonicWeight = 0 work, we need to adjust the code here: https://github.com/MTG/essentia/blob/master/src/algorithms/tonal/pitchsaliencefunction.cpp#L57
This is because the value of pow(0, 0) has undefined behavior. We should manually set the pow(0,0) value to 1. We can do it after the loop, setting _harmonicWeights[0] = 1 in the case if _harmonicWeight == 0.
There was a problem hiding this comment.
I have addressed the other comments and will make a local commit on them.
This comment requires more work.
I will propose to close this PR and I split this PR into two parts:
1 part taking 3 algorithsm and other part taking 2 algorithms
| filename = join(testdata.audio_dir, 'recorded', 'vignesh.wav') | ||
| audio = MonoLoader(filename=filename, sampleRate=44100)() | ||
| # Get the frequencies and magnitudes of the spectral peaks | ||
| freq_speaks, mag_speaks = SpectralPeaks()(audio) | ||
| # Start with default params | ||
| psf = PitchSalienceFunction() | ||
| calculatedPitchSalience = psf(freq_speaks,mag_speaks) | ||
|
|
There was a problem hiding this comment.
This is a wrong way of using SpectralPeaks. It is computed on spectrum on a frame level. Have a look at the example here: https://github.com/MTG/essentia/blob/master/src/examples/tutorial/example_predominantmelody_by_steps.py#L65
There was a problem hiding this comment.
PitchSalienceFunction is computed on separate frames. Thus, we can assess it on a few random frames, or the entire track, but only if the reference NPY array is not very large.
| frameSize = 1024 | ||
| hopSize = 512 |
There was a problem hiding this comment.
How do you decide on these parameters? The recommended settings in the DOC are:
Recommended processing chain: (see [1]): EqualLoudness -> frame slicing with sample rate = 44100, frame size = 2048, hop size = 128 -> Windowing with Hann, x4 zero padding -> Spectrum -> SpectralPeaks -> PitchSalienceFunction (10 cents bin resolution) -> PitchSalienceFunctionPeaks.
| def testARealCase(self): | ||
| frameSize = 1024 | ||
| sr = 44100 | ||
| hopSize = 512 |
There was a problem hiding this comment.
Same here. We could go for the default recommended settings instead.
|
replaced by #1142 and another new PR will be made for pitch contours |
Adding new scripts for testing pitch salience.
They contain the regular spot checks testInvalidParam, testEmpty, etc and one " testARealCase" with default params.
This initial draft is based on using as real input the audio file "vignesh.wav". This may be supplemented/replaced with other audio test files or with synthetic files to get more test coverage. The "testARealCase" is really a regression test using previous snapshot measurements as a baseline for detecting if future changes modify algorithm behaviour.