sof icon indicating copy to clipboard operation
sof copied to clipboard

Math: Add window functions library

Open singalsu opened this issue 2 years ago • 4 comments

This PR contains commits for library add and cmocka unit tests.

singalsu avatar Aug 12 '22 15:08 singalsu

This PR tests idea of executing Octave reference code to create test vector to compare. Unfortunately it's not possible to call host shell correctly from xt-run environment so it's left to only host unit tests (scripts/run-mocks.sh). Please let me know what you think. If not preferred, need to export to C header files the pre-computed reference windows coefficients.

singalsu avatar Aug 12 '22 15:08 singalsu

Looks good to me. And I'm curious about the rms between ref-win and win.

andrula-song avatar Aug 16 '22 08:08 andrula-song

Looks good to me. And I'm curious about the rms between ref-win and win.

I'm not sure if RMS is the best error criteria for this but it's simple. Here's output from ctest --verbose -R window. When I plotted difference there were a number of one LSB differences in Blackman and Povey.

51: Test command: /home/singalsu/work/current/sof/sof/build_ut/test/cmocka/src/math/window/window
51: Test timeout computed to be: 10000000
51: 1..4
51: system: cd /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window; octave --quiet --eval "ref_window('rectangular', 'window.txt', 16);"
51: Open file /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window/window.txt
51: Window rectangular RMS error = 0.000000
51: ok 1 - test_math_window_rectangular
51: system: cd /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window; octave --quiet --eval "ref_window('blackman', 'window.txt', 160);"
51: Open file /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window/window.txt
51: Window blackman RMS error = 0.000089
51: ok 2 - test_math_window_blackman
51: system: cd /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window; octave --quiet --eval "ref_window('hamming', 'window.txt', 256);"
51: Open file /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window/window.txt
51: Window hamming RMS error = 0.000000
51: ok 3 - test_math_window_hamming
51: system: cd /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window; octave --quiet --eval "ref_window('povey', 'window.txt', 200);"
51: Open file /home/singalsu/work/current/sof/sof/test/cmocka/src/math/window/window.txt
51: Window povey RMS error = 0.000009
51: ok 4 - test_math_window_povey
51: # ok - tests

Unfortunately I seem to need to change the test to be done with pre-calculated windows data due to unit test run fail above.

singalsu avatar Aug 17 '22 10:08 singalsu

The just updated version is using the old way pre-calculated data header files. The script ref_window.m was converted to export the headers instead of computing them in unit test runtime.

singalsu avatar Aug 17 '22 14:08 singalsu

@ShriramShastry good for you ?

Yes, it seems like the code is correct right now. In the present, as I return a second time, I see Error correction in Blackman window arithmetic should be improved.

ShriramShastry avatar Aug 24 '22 16:08 ShriramShastry

@ShriramShastry good for you ?

Yes, it seems like the code is correct right now. In the present, as I return a second time, I see Error correction in Blackman window arithmetic should be improved.

I see two improvements possibilities. The blackman computation could be changed to all 32 bits. Also the exp function used in povey could be replaced with better in future. But I propose to do the updates later. The difference vs. Matlab seen now is not impacting much MFCC output. I will also add more window functions later to better match MFCC options in Pytorch.

singalsu avatar Aug 25 '22 07:08 singalsu

@ShriramShastry good for you ?

Yes, it seems like the code is correct right now. In the present, as I return a second time, I see Error correction in Blackman window arithmetic should be improved.

I see two improvements possibilities. The blackman computation could be changed to all 32 bits. Also the exp function used in povey could be replaced with better in future. But I propose to do the updates later. The difference vs. Matlab seen now is not impacting much MFCC output. I will also add more window functions later to better match MFCC options in Pytorch.

@ShriramShastry good for you ?

Yes, it seems like the code is correct right now. In the present, as I return a second time, I see Error correction in Blackman window arithmetic should be improved.

I see two improvements possibilities. The blackman computation could be changed to all 32 bits. Also the exp function used in povey could be replaced with better in future. But I propose to do the updates later. The difference vs. Matlab seen now is not impacting much MFCC output. I will also add more window functions later to better match MFCC options in Pytorch.

I'm okay with the situation as it is now and with your new suggestion for improvement. Thanks

ShriramShastry avatar Aug 25 '22 09:08 ShriramShastry

This push fixed the conflict in test/cmocka/src/math/CMakeLists.txt, no other changes.

singalsu avatar Aug 26 '22 11:08 singalsu

The two codestyle checpatch fails are due to FILE *fh; The checker does not know the syntax. There's a debug option in unit test to output the numbers test data as text file for easy import to Matlab. Also can't avoid the CamelCase for CMUnitTest.

singalsu avatar Aug 26 '22 13:08 singalsu