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

Serious bug in the vlogq_neon_f32 code #13

Open
Gao-HaoYuan opened this issue Jan 26, 2024 · 4 comments
Open

Serious bug in the vlogq_neon_f32 code #13

Gao-HaoYuan opened this issue Jan 26, 2024 · 4 comments

Comments

@Gao-HaoYuan
Copy link

Gao-HaoYuan commented Jan 26, 2024

I have identified a bug in the vlogq_neon_f32 code. When calculating values close to 1, a significant error occurs, potentially leading to the inversion of the sign bit.

` float a = 0.1;
for(int i = 0; i < 9; ++i){
a += 0.1;
}
printf("%.9f\n", a);
float32x4_t in = {0.9f, 1.000000119, 1.1f, 1.2f};
float32x4_t out = vlogq_f32(in);
// out = vmulq_n_f32(out, 65535.f);
for(int i = 0; i < 4; ++i){
printf("%.9f\t", out[i]);
}
printf("\n");

for (int i = 0; i < 4; i++) {
    printf("%.9f\t", std::log(in[i]));
}
printf("\n");`

How to obtain correct results

@christophe0606
Copy link
Collaborator

@Gao-HaoYuan Is it a problem in CMSIS-DSP ? Or is it in a copy of this function used somewhere else ?

If it is the original function from CMSIS-DSP, please can you open an issue in https://github.com/ARM-software/CMSIS-DSP

@Gao-HaoYuan
Copy link
Author

hello @christophe0606 , This is the result of me copying the function and running it separately. I attempted to comprehend the log function's Taylor expansion around (x-n), but I couldn't ascertain the value of n. I discovered that the source of the error lies in the Taylor expansion function.
float32x4_t res = vmlaq_f32(vmlaq_f32(A, B, x2), vmlaq_f32(C, D, x2), x4);

The result of vmlaq_f32(C, D, x2), x4) is a negative number with an absolute value greater than vmlaq_f32(A, B, x2). This leads to the occurrence of -0 when calculating log(1).

and if this issue has been fixed in CMSIS-DSP?

@christophe0606
Copy link
Collaborator

@Gao-HaoYuan I just had a quick look at CMSIS-DSP and I think for Neon there is the problem. A Taylor expansion for the log has a limited validity so probably the argument should be scaled before and an offset added to the result when the argument is not in the right range.

I have created an issue : ARM-software/CMSIS-DSP#151

But unfortunately I won't be able to work on it quickly.

@Gao-HaoYuan
Copy link
Author

Gao-HaoYuan commented Jan 30, 2024

@christophe0606 I've conducted some tests, and when the value is very close to 1, there are significant differences in the calculation results. It might be beneficial to pay attention to values close to 1, such as 1.000000119, 1.00000119, and 0.9999911.

I attempted to compile and use CMSIS-DSP, but encountered some issues. If I manage to successfully compile it, I can also conduct tests within CMSIS-DSP.

However, I extracted the code for vlogq_f32 from CMSIS-DSP, and there are slight differences compared to EndpointAI's log_tab. When calculating log(1.000000119), the result is still -0 instead of +0.

That's the issue I've currently identified. I've also left a comment about this on ARM-software/CMSIS-DSP#151

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

No branches or pull requests

2 participants