search

ARM-software / CMSIS_5

CMSIS Version 5 Development Repository
http://arm-software.github.io/CMSIS_5/index.html
Apache License 2.0
1.33k stars 1.08k forks source link

Loss of precision in arm_cmplx_mag_q15. #566

Closed caseymillsdavis closed 4 years ago

caseymillsdavis commented 5 years ago

I noticed large discrepancies between the absolute value of the results of an fft using q15s versus a MATLAB reference implementation and the arm float implementation. I traced this down to a loss of precision occurring in arm_cmplx_mag_q15 (run after the fft).

    in = read_q15x2_ia ((q15_t **) &pSrc);
    acc0 = __SMUAD(in, in);
    /* store result in 2.14 format in destination buffer. */
    arm_sqrt_q15((q15_t) (acc0 >> 17), pDst++);

This shift before taking the square root is throwing out 8 bits of precision. For example a frequency bin with value 256 + 256i will have resultant magnitude of 0 with this computation.

caseymillsdavis commented 5 years ago

Isn't the comment incorrect as well? __SMUAD has squared two q15s, yielding two q30s, and then added them together. So the result is a q30, which when shifted by 17, will yield a q13, not q14.

christophe0606 commented 5 years ago

We definitely need to look at it. Thanks for reporting it.

christophe0606 commented 4 years ago

The fft Q15 have other accuracy issues.

arm_cmplx_mag_q15 is behaving as expected. It is not the most accurate implementation but it is not an outlier compared to other q15 functions of the CMSIS-DSP.

The comment is correct. The result of the multiply shifted by 17 is q13. sqrt is assuming a q15 in input. So there is an implied factor of 4 on the entry translating into a factor of 2 at the output.

Output of sqrt is assumed to be q15 too so with the implied factor of 2, the output is 2.14.