[FFmpeg-devel] checkasm: af_afir: Use a dynamic tolerance depending on values

As the values generated by av_bmg_get can be arbitrarily large
(only the stddev is specified), we can't use a fixed tolerance.
Calculate a dynamic tolerance (like in float_dsp from 38f966b2222db),
based on the individual steps of the calculation.

This fixes running this test with certain seeds, when built with
clang for mingw/x86_32.
---
 tests/checkasm/af_afir.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

On Wed, 11 Dec 2019, Martin Storsjö wrote:

> As the values generated by av_bmg_get can be arbitrarily large
> (only the stddev is specified), we can't use a fixed tolerance.
> Calculate a dynamic tolerance (like in float_dsp from 38f966b2222db),
> based on the individual steps of the calculation.
>
> This fixes running this test with certain seeds, when built with
> clang for mingw/x86_32.
> ---
> tests/checkasm/af_afir.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/tests/checkasm/af_afir.c b/tests/checkasm/af_afir.c
> index e3fb76e8e0..e791f88b97 100644
> --- a/tests/checkasm/af_afir.c
> +++ b/tests/checkasm/af_afir.c
> @@ -53,7 +53,19 @@ static void test_fcmul_add(const float *src0, const float *src1, const float *sr
>     call_ref(cdst, src1, src2, LEN);
>     call_new(odst, src1, src2, LEN);
>     for (i = 0; i <= LEN*2; i++) {
> -        if (!float_near_abs_eps(cdst[i], odst[i], 6.2e-05)) {
> +        int idx = i & ~1;
> +        float cre = src2[idx];
> +        float cim = src2[idx + 1];
> +        float tre = src1[idx];
> +        float tim = src1[idx + 1];
> +        double t = fabs(src0[i]) +
> +                   fabs(tre) + fabs(tim) + fabs(cre) + fabs(tim) +
> +                   fabs(tre * cre) + fabs(tim * cim) +
> +                   fabs(tre * cim) + fabs(tim * cre) +
> +                   fabs(tre * cre - tim * cim) +
> +                   fabs(tre * cim + tim * cre) +
> +                   fabs(cdst[i]) + 1.0;
> +        if (!float_near_abs_eps(cdst[i], odst[i], t * 2 * FLT_EPSILON)) {
>             fprintf(stderr, "%d: %- .12f - %- .12f = % .12g\n",
>                     i, cdst[i], odst[i], cdst[i] - odst[i]);
>             fail();
> -- 
> 2.17.1

Any comments about this one? It's the same kind of change as the float_dsp 
fix that was already approved and pushed, but this one is a bit larger as 
it's a much larger expression.

// Martin

On Wed, Dec 11, 2019 at 11:21:27AM +0200, Martin Storsjö wrote:
> As the values generated by av_bmg_get can be arbitrarily large
> (only the stddev is specified), we can't use a fixed tolerance.
> Calculate a dynamic tolerance (like in float_dsp from 38f966b2222db),
> based on the individual steps of the calculation.
> 
> This fixes running this test with certain seeds, when built with
> clang for mingw/x86_32.
> ---
>  tests/checkasm/af_afir.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/checkasm/af_afir.c b/tests/checkasm/af_afir.c
> index e3fb76e8e0..e791f88b97 100644
> --- a/tests/checkasm/af_afir.c
> +++ b/tests/checkasm/af_afir.c
> @@ -53,7 +53,19 @@ static void test_fcmul_add(const float *src0, const float *src1, const float *sr
>      call_ref(cdst, src1, src2, LEN);
>      call_new(odst, src1, src2, LEN);
>      for (i = 0; i <= LEN*2; i++) {
> -        if (!float_near_abs_eps(cdst[i], odst[i], 6.2e-05)) {
> +        int idx = i & ~1;
> +        float cre = src2[idx];
> +        float cim = src2[idx + 1];
> +        float tre = src1[idx];
> +        float tim = src1[idx + 1];
> +        double t = fabs(src0[i]) +

> +                   fabs(tre) + fabs(tim) + fabs(cre) + fabs(tim) +

Is it intended to add fabs(tim) twice ?

thx

[...]

On Thu, 12 Dec 2019, Michael Niedermayer wrote:

> On Wed, Dec 11, 2019 at 11:21:27AM +0200, Martin Storsjö wrote:
>> As the values generated by av_bmg_get can be arbitrarily large
>> (only the stddev is specified), we can't use a fixed tolerance.
>> Calculate a dynamic tolerance (like in float_dsp from 38f966b2222db),
>> based on the individual steps of the calculation.
>>
>> This fixes running this test with certain seeds, when built with
>> clang for mingw/x86_32.
>> ---
>>  tests/checkasm/af_afir.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/checkasm/af_afir.c b/tests/checkasm/af_afir.c
>> index e3fb76e8e0..e791f88b97 100644
>> --- a/tests/checkasm/af_afir.c
>> +++ b/tests/checkasm/af_afir.c
>> @@ -53,7 +53,19 @@ static void test_fcmul_add(const float *src0, const float *src1, const float *sr
>>      call_ref(cdst, src1, src2, LEN);
>>      call_new(odst, src1, src2, LEN);
>>      for (i = 0; i <= LEN*2; i++) {
>> -        if (!float_near_abs_eps(cdst[i], odst[i], 6.2e-05)) {
>> +        int idx = i & ~1;
>> +        float cre = src2[idx];
>> +        float cim = src2[idx + 1];
>> +        float tre = src1[idx];
>> +        float tim = src1[idx + 1];
>> +        double t = fabs(src0[i]) +
>
>> +                   fabs(tre) + fabs(tim) + fabs(cre) + fabs(tim) +
>
> Is it intended to add fabs(tim) twice ?

Oops, that's a typo, it was meant to be tre+tim+cre+cim.

// Martin