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

What is the meaning of LOG2TOLOG_Q15 constant? #107

Closed
olegkhr opened this issue Jun 9, 2023 · 10 comments
Closed

What is the meaning of LOG2TOLOG_Q15 constant? #107

olegkhr opened this issue Jun 9, 2023 · 10 comments
Labels
bug Something isn't working DONE Issue done but not yet closed

Comments

@olegkhr
Copy link

olegkhr commented Jun 9, 2023

No description provided.

@olegkhr
Copy link
Author

olegkhr commented Jun 10, 2023

why is it equal to LOG2TOLOG_Q31?

@christophe0606 christophe0606 added the review Under review label Jun 12, 2023
@christophe0606
Copy link
Contributor

@olegkhr It is to convert a log2 value to a normal log (base e)
Initially the q15 version was using a q15 log. It was not accurate enough. So internally now the q15 version is also using a q31 log. As consequence, the two values are now equal.
The naming is a bit confusing since the Q31 or Q15 in the name is not referring to the format of the constant but to the format of the function.

@christophe0606 christophe0606 added question Further information is requested and removed review Under review labels Jun 12, 2023
@olegkhr
Copy link
Author

olegkhr commented Jun 12, 2023

@christophe0606 thanks for the explanation.

For the very low-power signal - mel spectrum is computed as all less than 1 values. Then vlog will output negative values.
But then after applying offset all the values become positive: https://github.com/ARM-software/CMSIS-DSP/blob/main/Source/TransformFunctions/arm_mfcc_q15.c#L179.

That results in a large difference in the first coefficient in DCT between float and quant versions.

Is it expected behavior?

@christophe0606
Copy link
Contributor

@olegkhr This offset is required because of the shifts occurring in fixed point.
Q15 will never be able to be close to f32 for small signals but the tradeoff saturation / accuracy of the algorithm may be changed a little.

Can you share an example signal and MFCC parameters so that I can't look at it.
Or better, do you have a Python to reproduce the problem ?

You can have a look here : #54 (comment)

I had written some Python to compare CMSIS-DSP MFCC, TensorFlow MFCC and Librosa MFCC.

I you share a f32 version, I can make the q15 to compare and see what's happening.
But just a example signal and the MFCC params would be enough.

@olegkhr
Copy link
Author

olegkhr commented Jun 21, 2023

MFCC params:

sample_rate = 16000
FFTSize = 512
numOfDctOutputs = 49

freq_min = 20
freq_high = 7000
numOfMelFilters = 60

Example signal:
[ 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 0 0 0 0 0 -1 0 -1 0 -1 0 -3 -4 -4 -4 -5 -5 -4 -5
-5 -5 -5 -6 -6 -7 -7 -7 -7 -7 -7 -7 -8 -8 -8 -8 -8 -7 -7 -7 -6 -6 -6 -6
-6 -7 -7 -7 -6 -6 -6 -6 -6 -6 -6 -7 -7 -7 -8 -7 -7 -6 -6 -6 -6 -7 -7 -7
-7 -7 -6 -6 -6 -5 -6 -5 -5 -5 -6 -5 -5 -5 -5 -5 -5 -5 -5 -5 -6 -6 -6 -6
-6 -6 -6 -7 -7 -7 -6 -7 -7 -7 -7 -6 -6 -5 -5 -4 -4 -4 -4 -4 -4 -4 -4 -3
-3 -3 -3 -3 -3 -3 -3 -3 -3 -3 -3 -4 -3 -3 -3 -3 -3 -3 -3 -3 -2 -1 -1 -1
-1 -1 -1 -1 -1 -1 0 -1 0 0 0 0 0 0 -1 -1 -2 -2 -2 -3 -3 -3 -3 -3
-4 -4 -4 -4 -4 -3 -3 -3 -2 -2 -2 -2 -3 -3 -3 -3 -3 -3 -3 -3 -3 -3 -3 -4
-4 -4 -4 -5 -5 -5 -4 -4 -4 -4 -5 -5 -5 -4 -4 -4 -3 -3 -3 -3 -3 -3 -3 -3
-3 -3 -4 -3 -3 -3 -4 -4 -4 -4 -5 -5 -5 -5 -4 -4 -4 -5 -5 -5 -5 -5 -5 -5
-4 -3 -2 -2 -2 -2 -3 -3 -3 -3 -2 -2 -1 -1 -1 -1 -1 -1 -2 -2 -2 -2 -2 -2
-2 -1 -1 -1 -1 -1 -1 -1 0 0 1 1 1 2 1 2 2 1 1 1 1 1 1 1
2 2 2 1 1 1 1 1 2 1 2 2 1 1 1 1 1 1 2 2 3 3 3 3
3 3 3 3 3 2 3 4 4 4 4 4 3 3 3 3 3 3 3 3 3 4 4 4
3 4 4 4 5 5 5 6 5 5 5 6 6 5 5 5 6 6 6 6 5 4 4 4
5 4 4 4 4 3 3 3 2 2 3 2 3 3 4 4 5 5 4 3 4 4 4 4
4 4 5 4 4 4 4 4 3 3 3 3 3 3 4 4 4 4 4 4 4 4 5 5
5 5 6 6 6 6 6 5 6 5 6 6 6 6 6 6 5 5 5 5 5 5 6 6
5 5 5 6 5 5 5 6 6 6 7 7 7 7 7 7 7 7 7 8 8 8 8 8
7 8 7 7 7 7 8 7]

@olegkhr
Copy link
Author

olegkhr commented Jun 26, 2023

@christophe0606
Copy link
Contributor

@olegkhr Sorry for late answer. I was off for some time. I'll look at all of this as soon as I can.

@olegkhr
Copy link
Author

olegkhr commented Jul 19, 2023

@christophe0606 Could you please review this patch?
With this patch I am getting much better match:

diff --git a/Source/TransformFunctions/arm_mfcc_q15.c b/Source/TransformFunctions/arm_mfcc_q15.c
index 7512afd1..3668585f 100755
--- a/Source/TransformFunctions/arm_mfcc_q15.c
+++ b/Source/TransformFunctions/arm_mfcc_q15.c
@@ -96,10 +96,10 @@ arm_status arm_mfcc_q15(
     // q15
     arm_absmax_q15(pSrc,S->fftLen,&m,&index);

+    int16_t shift;
     if (m !=0)
     {
        q15_t quotient;
-       int16_t shift;

        status = arm_divide_q15(0x7FFF,m,&quotient,&shift);
        if (status != ARM_MATH_SUCCESS)
@@ -171,7 +171,7 @@ arm_status arm_mfcc_q15(

     // q5.26

-    logExponent = fftShift + 2 + SHIFT_MELFILTER_SATURATION_Q15;
+    logExponent = fftShift + 2 + SHIFT_MELFILTER_SATURATION_Q15 - shift;
     logExponent = logExponent * LOG2TOLOG_Q15;

@christophe0606
Copy link
Contributor

@olegkhr I think you have found a problem. The signal is rescaled to have more accuracy for the FFT and magnitude computation but the rescaling is not compensated later.

The patch above is identifying the problem but not applying the rescaling compensation where it should. I'll look at it.

Q31 also has the problem.

Thanks for reporting it and thanks for being patient because I have been very slow at looking at this github issue.

@christophe0606 christophe0606 added bug Something isn't working and removed question Further information is requested labels Jul 19, 2023
@christophe0606
Copy link
Contributor

@olegkhr The latest commit 60a8c88 is correcting a rescaling issue in all MFCCs versions.

Tell me if it solves your problem.

I observe that the MFCC Q15 is closer to the other versions with this fix.

Also, the MFCC F32 is closer to the TensorFlow one (I was observing a small divergence on a test pattern and it is now corrected with this commit).

@christophe0606 christophe0606 added the DONE Issue done but not yet closed label Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working DONE Issue done but not yet closed
Projects
None yet
Development

No branches or pull requests

2 participants