Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34
       [not found] <20231211164227.163BB4115B0@natalya.videolan.org>
@ 2024-01-23 19:07 ` Michael Niedermayer
  2024-01-23 19:17   ` James Almer
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Niedermayer @ 2024-01-23 19:07 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1: Type: text/plain, Size: 2321 bytes --]

On Mon, Dec 11, 2023 at 04:42:26PM +0000, sunyuechi wrote:
> ffmpeg | branch: master | sunyuechi <sunyuechi@iscas.ac.cn> | Tue Nov 28 14:08:12 2023 +0800| [1c3620b2bbe73db9239fcf605e8f535b58f03b86] | committer: Rémi Denis-Courmont
>
> checkasm: test for abs_pow34
>
> Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
>
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1c3620b2bbe73db9239fcf605e8f535b58f03b86
> ---
>
>  tests/checkasm/Makefile    |  1 +
>  tests/checkasm/aacencdsp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/checkasm/checkasm.c  |  3 ++
>  tests/checkasm/checkasm.h  |  1 +
>  tests/fate/checkasm.mak    |  3 +-
>  5 files changed, 77 insertions(+), 1 deletion(-)
[...]
> +static void test_abs_pow34(AACEncContext *s) {
> +#define BUF_SIZE 1024
> +    LOCAL_ALIGNED_32(float, in, [BUF_SIZE]);
> +
> +    declare_func(void, float *, const float *, int);
> +
> +    randomize_float(in, BUF_SIZE);
> +
> +    if (check_func(s->abs_pow34, "abs_pow34")) {
> +        LOCAL_ALIGNED_32(float, out, [BUF_SIZE]);
> +        LOCAL_ALIGNED_32(float, out2, [BUF_SIZE]);
> +
> +        call_ref(out, in, BUF_SIZE);
> +        call_new(out2, in, BUF_SIZE);
> +
> +        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> +            fail();

This is wrong if one of the functions is implemented in C
C does not specify float to give bitexact results. Any equality check with
float thats expected to end in a reproduceable outcome is generally wrong

it also fails in practice

TEST    checkasm-aacencdsp
Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
src/tests/Makefile:308: recipe for target 'fate-checkasm-aacencdsp' failed
make: *** [fate-checkasm-aacencdsp] Error 1

checkasm: using random seed 2523748868
SSE:
   abs_pow34_sse (aacencdsp.c:55)
 - aacencdsp.abs_pow34 [FAILED]
checkasm: 1 of 1 tests have failed
threads=1

thats on plain simple x86-32


thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34
  2024-01-23 19:07 ` [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34 Michael Niedermayer
@ 2024-01-23 19:17   ` James Almer
  2024-01-23 19:33     ` Michael Niedermayer
  0 siblings, 1 reply; 7+ messages in thread
From: James Almer @ 2024-01-23 19:17 UTC (permalink / raw)
  To: ffmpeg-devel

On 1/23/2024 4:07 PM, Michael Niedermayer wrote:
> On Mon, Dec 11, 2023 at 04:42:26PM +0000, sunyuechi wrote:
>> ffmpeg | branch: master | sunyuechi <sunyuechi@iscas.ac.cn> | Tue Nov 28 14:08:12 2023 +0800| [1c3620b2bbe73db9239fcf605e8f535b58f03b86] | committer: Rémi Denis-Courmont
>>
>> checkasm: test for abs_pow34
>>
>> Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1c3620b2bbe73db9239fcf605e8f535b58f03b86
>> ---
>>
>>   tests/checkasm/Makefile    |  1 +
>>   tests/checkasm/aacencdsp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/checkasm/checkasm.c  |  3 ++
>>   tests/checkasm/checkasm.h  |  1 +
>>   tests/fate/checkasm.mak    |  3 +-
>>   5 files changed, 77 insertions(+), 1 deletion(-)
> [...]
>> +static void test_abs_pow34(AACEncContext *s) {
>> +#define BUF_SIZE 1024
>> +    LOCAL_ALIGNED_32(float, in, [BUF_SIZE]);
>> +
>> +    declare_func(void, float *, const float *, int);
>> +
>> +    randomize_float(in, BUF_SIZE);
>> +
>> +    if (check_func(s->abs_pow34, "abs_pow34")) {
>> +        LOCAL_ALIGNED_32(float, out, [BUF_SIZE]);
>> +        LOCAL_ALIGNED_32(float, out2, [BUF_SIZE]);
>> +
>> +        call_ref(out, in, BUF_SIZE);
>> +        call_new(out2, in, BUF_SIZE);
>> +
>> +        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
>> +            fail();
> 
> This is wrong if one of the functions is implemented in C
> C does not specify float to give bitexact results. Any equality check with
> float thats expected to end in a reproduceable outcome is generally wrong

Yes, that's why float_near_abs_eps_array() or float_near_ulp_array() 
should be used.

> 
> it also fails in practice
> 
> TEST    checkasm-aacencdsp
> Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
> src/tests/Makefile:308: recipe for target 'fate-checkasm-aacencdsp' failed
> make: *** [fate-checkasm-aacencdsp] Error 1
> 
> checkasm: using random seed 2523748868
> SSE:
>     abs_pow34_sse (aacencdsp.c:55)
>   - aacencdsp.abs_pow34 [FAILED]
> checkasm: 1 of 1 tests have failed
> threads=1
> 
> thats on plain simple x86-32

Can you check if the following passes for x86-32?

> diff --git a/tests/checkasm/aacencdsp.c b/tests/checkasm/aacencdsp.c
> index 684c775862..7cc8a01158 100644
> --- a/tests/checkasm/aacencdsp.c
> +++ b/tests/checkasm/aacencdsp.c
> @@ -19,6 +19,7 @@
>   */
> 
>  #include <string.h>
> +#include <float.h>
> 
>  #include "libavutil/mem.h"
>  #include "libavutil/mem_internal.h"
> @@ -51,7 +52,7 @@ static void test_abs_pow34(AACEncContext *s) {
>          call_ref(out, in, BUF_SIZE);
>          call_new(out2, in, BUF_SIZE);
> 
> -        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> +        if (!float_near_abs_eps_array(out, out2, FLT_EPSILON, BUF_SIZE))
>              fail();
> 
>          bench_new(out, in, BUF_SIZE);

That target tends to be pretty picky about the epsilon value because of 
the 80 bits precision for floats.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34
  2024-01-23 19:17   ` James Almer
@ 2024-01-23 19:33     ` Michael Niedermayer
  2024-01-23 19:40       ` James Almer
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Niedermayer @ 2024-01-23 19:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 4274 bytes --]

On Tue, Jan 23, 2024 at 04:17:22PM -0300, James Almer wrote:
> On 1/23/2024 4:07 PM, Michael Niedermayer wrote:
> > On Mon, Dec 11, 2023 at 04:42:26PM +0000, sunyuechi wrote:
> > > ffmpeg | branch: master | sunyuechi <sunyuechi@iscas.ac.cn> | Tue Nov 28 14:08:12 2023 +0800| [1c3620b2bbe73db9239fcf605e8f535b58f03b86] | committer: Rémi Denis-Courmont
> > > 
> > > checkasm: test for abs_pow34
> > > 
> > > Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
> > > 
> > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1c3620b2bbe73db9239fcf605e8f535b58f03b86
> > > ---
> > > 
> > >   tests/checkasm/Makefile    |  1 +
> > >   tests/checkasm/aacencdsp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> > >   tests/checkasm/checkasm.c  |  3 ++
> > >   tests/checkasm/checkasm.h  |  1 +
> > >   tests/fate/checkasm.mak    |  3 +-
> > >   5 files changed, 77 insertions(+), 1 deletion(-)
> > [...]
> > > +static void test_abs_pow34(AACEncContext *s) {
> > > +#define BUF_SIZE 1024
> > > +    LOCAL_ALIGNED_32(float, in, [BUF_SIZE]);
> > > +
> > > +    declare_func(void, float *, const float *, int);
> > > +
> > > +    randomize_float(in, BUF_SIZE);
> > > +
> > > +    if (check_func(s->abs_pow34, "abs_pow34")) {
> > > +        LOCAL_ALIGNED_32(float, out, [BUF_SIZE]);
> > > +        LOCAL_ALIGNED_32(float, out2, [BUF_SIZE]);
> > > +
> > > +        call_ref(out, in, BUF_SIZE);
> > > +        call_new(out2, in, BUF_SIZE);
> > > +
> > > +        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> > > +            fail();
> > 
> > This is wrong if one of the functions is implemented in C
> > C does not specify float to give bitexact results. Any equality check with
> > float thats expected to end in a reproduceable outcome is generally wrong
> 
> Yes, that's why float_near_abs_eps_array() or float_near_ulp_array() should
> be used.
> 
> > 
> > it also fails in practice
> > 
> > TEST    checkasm-aacencdsp
> > Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
> > src/tests/Makefile:308: recipe for target 'fate-checkasm-aacencdsp' failed
> > make: *** [fate-checkasm-aacencdsp] Error 1
> > 
> > checkasm: using random seed 2523748868
> > SSE:
> >     abs_pow34_sse (aacencdsp.c:55)
> >   - aacencdsp.abs_pow34 [FAILED]
> > checkasm: 1 of 1 tests have failed
> > threads=1
> > 
> > thats on plain simple x86-32
> 
> Can you check if the following passes for x86-32?
> 
> > diff --git a/tests/checkasm/aacencdsp.c b/tests/checkasm/aacencdsp.c
> > index 684c775862..7cc8a01158 100644
> > --- a/tests/checkasm/aacencdsp.c
> > +++ b/tests/checkasm/aacencdsp.c
> > @@ -19,6 +19,7 @@
> >   */
> > 
> >  #include <string.h>
> > +#include <float.h>
> > 
> >  #include "libavutil/mem.h"
> >  #include "libavutil/mem_internal.h"
> > @@ -51,7 +52,7 @@ static void test_abs_pow34(AACEncContext *s) {
> >          call_ref(out, in, BUF_SIZE);
> >          call_new(out2, in, BUF_SIZE);
> > 
> > -        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> > +        if (!float_near_abs_eps_array(out, out2, FLT_EPSILON, BUF_SIZE))
> >              fail();
> > 
> >          bench_new(out, in, BUF_SIZE);
> 
> That target tends to be pretty picky about the epsilon value because of the
> 80 bits precision for floats.


with float.h

float_near_abs_eps_array

Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
checkasm: using random seed 4097080137
test failed comparing 5.68635 with 5.68635 (abs diff=4.76837e-07 with EPS=1.19209e-07)
SSE:
   abs_pow34_sse (aacencdsp.c:56)
 - aacencdsp.abs_pow34 [FAILED]
checkasm: 1 of 1 tests have failed
threads=1
src/tests/Makefile:317: recipe for target 'fate-checkasm-aacencdsp' failed
make: *** [fate-checkasm-aacencdsp] Error 1

if (!float_near_abs_eps_array(out, out2, FLT_EPSILON*10, BUF_SIZE))
works but feels wrong

this works too (only tested 100x on only x86-32 yet)
if (!float_near_ulp_array(out, out2, 1, BUF_SIZE))

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34
  2024-01-23 19:33     ` Michael Niedermayer
@ 2024-01-23 19:40       ` James Almer
  2024-01-23 19:49         ` Michael Niedermayer
  0 siblings, 1 reply; 7+ messages in thread
From: James Almer @ 2024-01-23 19:40 UTC (permalink / raw)
  To: ffmpeg-devel

On 1/23/2024 4:33 PM, Michael Niedermayer wrote:
> On Tue, Jan 23, 2024 at 04:17:22PM -0300, James Almer wrote:
>> On 1/23/2024 4:07 PM, Michael Niedermayer wrote:
>>> On Mon, Dec 11, 2023 at 04:42:26PM +0000, sunyuechi wrote:
>>>> ffmpeg | branch: master | sunyuechi <sunyuechi@iscas.ac.cn> | Tue Nov 28 14:08:12 2023 +0800| [1c3620b2bbe73db9239fcf605e8f535b58f03b86] | committer: Rémi Denis-Courmont
>>>>
>>>> checkasm: test for abs_pow34
>>>>
>>>> Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
>>>>
>>>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1c3620b2bbe73db9239fcf605e8f535b58f03b86
>>>> ---
>>>>
>>>>    tests/checkasm/Makefile    |  1 +
>>>>    tests/checkasm/aacencdsp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>    tests/checkasm/checkasm.c  |  3 ++
>>>>    tests/checkasm/checkasm.h  |  1 +
>>>>    tests/fate/checkasm.mak    |  3 +-
>>>>    5 files changed, 77 insertions(+), 1 deletion(-)
>>> [...]
>>>> +static void test_abs_pow34(AACEncContext *s) {
>>>> +#define BUF_SIZE 1024
>>>> +    LOCAL_ALIGNED_32(float, in, [BUF_SIZE]);
>>>> +
>>>> +    declare_func(void, float *, const float *, int);
>>>> +
>>>> +    randomize_float(in, BUF_SIZE);
>>>> +
>>>> +    if (check_func(s->abs_pow34, "abs_pow34")) {
>>>> +        LOCAL_ALIGNED_32(float, out, [BUF_SIZE]);
>>>> +        LOCAL_ALIGNED_32(float, out2, [BUF_SIZE]);
>>>> +
>>>> +        call_ref(out, in, BUF_SIZE);
>>>> +        call_new(out2, in, BUF_SIZE);
>>>> +
>>>> +        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
>>>> +            fail();
>>>
>>> This is wrong if one of the functions is implemented in C
>>> C does not specify float to give bitexact results. Any equality check with
>>> float thats expected to end in a reproduceable outcome is generally wrong
>>
>> Yes, that's why float_near_abs_eps_array() or float_near_ulp_array() should
>> be used.
>>
>>>
>>> it also fails in practice
>>>
>>> TEST    checkasm-aacencdsp
>>> Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
>>> src/tests/Makefile:308: recipe for target 'fate-checkasm-aacencdsp' failed
>>> make: *** [fate-checkasm-aacencdsp] Error 1
>>>
>>> checkasm: using random seed 2523748868
>>> SSE:
>>>      abs_pow34_sse (aacencdsp.c:55)
>>>    - aacencdsp.abs_pow34 [FAILED]
>>> checkasm: 1 of 1 tests have failed
>>> threads=1
>>>
>>> thats on plain simple x86-32
>>
>> Can you check if the following passes for x86-32?
>>
>>> diff --git a/tests/checkasm/aacencdsp.c b/tests/checkasm/aacencdsp.c
>>> index 684c775862..7cc8a01158 100644
>>> --- a/tests/checkasm/aacencdsp.c
>>> +++ b/tests/checkasm/aacencdsp.c
>>> @@ -19,6 +19,7 @@
>>>    */
>>>
>>>   #include <string.h>
>>> +#include <float.h>
>>>
>>>   #include "libavutil/mem.h"
>>>   #include "libavutil/mem_internal.h"
>>> @@ -51,7 +52,7 @@ static void test_abs_pow34(AACEncContext *s) {
>>>           call_ref(out, in, BUF_SIZE);
>>>           call_new(out2, in, BUF_SIZE);
>>>
>>> -        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
>>> +        if (!float_near_abs_eps_array(out, out2, FLT_EPSILON, BUF_SIZE))
>>>               fail();
>>>
>>>           bench_new(out, in, BUF_SIZE);
>>
>> That target tends to be pretty picky about the epsilon value because of the
>> 80 bits precision for floats.
> 
> 
> with float.h
> 
> float_near_abs_eps_array
> 
> Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
> checkasm: using random seed 4097080137
> test failed comparing 5.68635 with 5.68635 (abs diff=4.76837e-07 with EPS=1.19209e-07)
> SSE:
>     abs_pow34_sse (aacencdsp.c:56)
>   - aacencdsp.abs_pow34 [FAILED]
> checkasm: 1 of 1 tests have failed
> threads=1
> src/tests/Makefile:317: recipe for target 'fate-checkasm-aacencdsp' failed
> make: *** [fate-checkasm-aacencdsp] Error 1
> 
> if (!float_near_abs_eps_array(out, out2, FLT_EPSILON*10, BUF_SIZE))
> works but feels wrong
> 
> this works too (only tested 100x on only x86-32 yet)
> if (!float_near_ulp_array(out, out2, 1, BUF_SIZE))
> 
> thx

1 is excessive. Some tests use an epsilon value like 0.005, so 
FLT_EPSILON*10 is IMO fine. Or we could hardcode something like 0.000005 
instead if it passes.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34
  2024-01-23 19:40       ` James Almer
@ 2024-01-23 19:49         ` Michael Niedermayer
  2024-01-23 19:52           ` James Almer
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Niedermayer @ 2024-01-23 19:49 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 5591 bytes --]

On Tue, Jan 23, 2024 at 04:40:46PM -0300, James Almer wrote:
> On 1/23/2024 4:33 PM, Michael Niedermayer wrote:
> > On Tue, Jan 23, 2024 at 04:17:22PM -0300, James Almer wrote:
> > > On 1/23/2024 4:07 PM, Michael Niedermayer wrote:
> > > > On Mon, Dec 11, 2023 at 04:42:26PM +0000, sunyuechi wrote:
> > > > > ffmpeg | branch: master | sunyuechi <sunyuechi@iscas.ac.cn> | Tue Nov 28 14:08:12 2023 +0800| [1c3620b2bbe73db9239fcf605e8f535b58f03b86] | committer: Rémi Denis-Courmont
> > > > > 
> > > > > checkasm: test for abs_pow34
> > > > > 
> > > > > Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
> > > > > 
> > > > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1c3620b2bbe73db9239fcf605e8f535b58f03b86
> > > > > ---
> > > > > 
> > > > >    tests/checkasm/Makefile    |  1 +
> > > > >    tests/checkasm/aacencdsp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >    tests/checkasm/checkasm.c  |  3 ++
> > > > >    tests/checkasm/checkasm.h  |  1 +
> > > > >    tests/fate/checkasm.mak    |  3 +-
> > > > >    5 files changed, 77 insertions(+), 1 deletion(-)
> > > > [...]
> > > > > +static void test_abs_pow34(AACEncContext *s) {
> > > > > +#define BUF_SIZE 1024
> > > > > +    LOCAL_ALIGNED_32(float, in, [BUF_SIZE]);
> > > > > +
> > > > > +    declare_func(void, float *, const float *, int);
> > > > > +
> > > > > +    randomize_float(in, BUF_SIZE);
> > > > > +
> > > > > +    if (check_func(s->abs_pow34, "abs_pow34")) {
> > > > > +        LOCAL_ALIGNED_32(float, out, [BUF_SIZE]);
> > > > > +        LOCAL_ALIGNED_32(float, out2, [BUF_SIZE]);
> > > > > +
> > > > > +        call_ref(out, in, BUF_SIZE);
> > > > > +        call_new(out2, in, BUF_SIZE);
> > > > > +
> > > > > +        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> > > > > +            fail();
> > > > 
> > > > This is wrong if one of the functions is implemented in C
> > > > C does not specify float to give bitexact results. Any equality check with
> > > > float thats expected to end in a reproduceable outcome is generally wrong
> > > 
> > > Yes, that's why float_near_abs_eps_array() or float_near_ulp_array() should
> > > be used.
> > > 
> > > > 
> > > > it also fails in practice
> > > > 
> > > > TEST    checkasm-aacencdsp
> > > > Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
> > > > src/tests/Makefile:308: recipe for target 'fate-checkasm-aacencdsp' failed
> > > > make: *** [fate-checkasm-aacencdsp] Error 1
> > > > 
> > > > checkasm: using random seed 2523748868
> > > > SSE:
> > > >      abs_pow34_sse (aacencdsp.c:55)
> > > >    - aacencdsp.abs_pow34 [FAILED]
> > > > checkasm: 1 of 1 tests have failed
> > > > threads=1
> > > > 
> > > > thats on plain simple x86-32
> > > 
> > > Can you check if the following passes for x86-32?
> > > 
> > > > diff --git a/tests/checkasm/aacencdsp.c b/tests/checkasm/aacencdsp.c
> > > > index 684c775862..7cc8a01158 100644
> > > > --- a/tests/checkasm/aacencdsp.c
> > > > +++ b/tests/checkasm/aacencdsp.c
> > > > @@ -19,6 +19,7 @@
> > > >    */
> > > > 
> > > >   #include <string.h>
> > > > +#include <float.h>
> > > > 
> > > >   #include "libavutil/mem.h"
> > > >   #include "libavutil/mem_internal.h"
> > > > @@ -51,7 +52,7 @@ static void test_abs_pow34(AACEncContext *s) {
> > > >           call_ref(out, in, BUF_SIZE);
> > > >           call_new(out2, in, BUF_SIZE);
> > > > 
> > > > -        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> > > > +        if (!float_near_abs_eps_array(out, out2, FLT_EPSILON, BUF_SIZE))
> > > >               fail();
> > > > 
> > > >           bench_new(out, in, BUF_SIZE);
> > > 
> > > That target tends to be pretty picky about the epsilon value because of the
> > > 80 bits precision for floats.
> > 
> > 
> > with float.h
> > 
> > float_near_abs_eps_array
> > 
> > Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
> > checkasm: using random seed 4097080137
> > test failed comparing 5.68635 with 5.68635 (abs diff=4.76837e-07 with EPS=1.19209e-07)
> > SSE:
> >     abs_pow34_sse (aacencdsp.c:56)
> >   - aacencdsp.abs_pow34 [FAILED]
> > checkasm: 1 of 1 tests have failed
> > threads=1
> > src/tests/Makefile:317: recipe for target 'fate-checkasm-aacencdsp' failed
> > make: *** [fate-checkasm-aacencdsp] Error 1
> > 
> > if (!float_near_abs_eps_array(out, out2, FLT_EPSILON*10, BUF_SIZE))
> > works but feels wrong
> > 
> > this works too (only tested 100x on only x86-32 yet)
> > if (!float_near_ulp_array(out, out2, 1, BUF_SIZE))
> > 
> > thx
> 
> 1 is excessive. Some tests use an epsilon value like 0.005, so
> FLT_EPSILON*10 is IMO fine. Or we could hardcode something like 0.000005
> instead if it passes.

1 ULP is the smallest that works
tested 100x on mips, arm, mingw32 now too

epsilon anything is IMHO wrong but iam happy to hear an argument why
absolute is better than relative here (which is the difference bascially
IIUC)

absolute is bad as the difference in what can be represented is relative
to the values size and so a absolute check will inevitably be very loose
on small values or will fail on large ones

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34
  2024-01-23 19:49         ` Michael Niedermayer
@ 2024-01-23 19:52           ` James Almer
  2024-01-23 20:09             ` Michael Niedermayer
  0 siblings, 1 reply; 7+ messages in thread
From: James Almer @ 2024-01-23 19:52 UTC (permalink / raw)
  To: ffmpeg-devel

On 1/23/2024 4:49 PM, Michael Niedermayer wrote:
> On Tue, Jan 23, 2024 at 04:40:46PM -0300, James Almer wrote:
>> On 1/23/2024 4:33 PM, Michael Niedermayer wrote:
>>> On Tue, Jan 23, 2024 at 04:17:22PM -0300, James Almer wrote:
>>>> On 1/23/2024 4:07 PM, Michael Niedermayer wrote:
>>>>> On Mon, Dec 11, 2023 at 04:42:26PM +0000, sunyuechi wrote:
>>>>>> ffmpeg | branch: master | sunyuechi <sunyuechi@iscas.ac.cn> | Tue Nov 28 14:08:12 2023 +0800| [1c3620b2bbe73db9239fcf605e8f535b58f03b86] | committer: Rémi Denis-Courmont
>>>>>>
>>>>>> checkasm: test for abs_pow34
>>>>>>
>>>>>> Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
>>>>>>
>>>>>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1c3620b2bbe73db9239fcf605e8f535b58f03b86
>>>>>> ---
>>>>>>
>>>>>>     tests/checkasm/Makefile    |  1 +
>>>>>>     tests/checkasm/aacencdsp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>     tests/checkasm/checkasm.c  |  3 ++
>>>>>>     tests/checkasm/checkasm.h  |  1 +
>>>>>>     tests/fate/checkasm.mak    |  3 +-
>>>>>>     5 files changed, 77 insertions(+), 1 deletion(-)
>>>>> [...]
>>>>>> +static void test_abs_pow34(AACEncContext *s) {
>>>>>> +#define BUF_SIZE 1024
>>>>>> +    LOCAL_ALIGNED_32(float, in, [BUF_SIZE]);
>>>>>> +
>>>>>> +    declare_func(void, float *, const float *, int);
>>>>>> +
>>>>>> +    randomize_float(in, BUF_SIZE);
>>>>>> +
>>>>>> +    if (check_func(s->abs_pow34, "abs_pow34")) {
>>>>>> +        LOCAL_ALIGNED_32(float, out, [BUF_SIZE]);
>>>>>> +        LOCAL_ALIGNED_32(float, out2, [BUF_SIZE]);
>>>>>> +
>>>>>> +        call_ref(out, in, BUF_SIZE);
>>>>>> +        call_new(out2, in, BUF_SIZE);
>>>>>> +
>>>>>> +        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
>>>>>> +            fail();
>>>>>
>>>>> This is wrong if one of the functions is implemented in C
>>>>> C does not specify float to give bitexact results. Any equality check with
>>>>> float thats expected to end in a reproduceable outcome is generally wrong
>>>>
>>>> Yes, that's why float_near_abs_eps_array() or float_near_ulp_array() should
>>>> be used.
>>>>
>>>>>
>>>>> it also fails in practice
>>>>>
>>>>> TEST    checkasm-aacencdsp
>>>>> Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
>>>>> src/tests/Makefile:308: recipe for target 'fate-checkasm-aacencdsp' failed
>>>>> make: *** [fate-checkasm-aacencdsp] Error 1
>>>>>
>>>>> checkasm: using random seed 2523748868
>>>>> SSE:
>>>>>       abs_pow34_sse (aacencdsp.c:55)
>>>>>     - aacencdsp.abs_pow34 [FAILED]
>>>>> checkasm: 1 of 1 tests have failed
>>>>> threads=1
>>>>>
>>>>> thats on plain simple x86-32
>>>>
>>>> Can you check if the following passes for x86-32?
>>>>
>>>>> diff --git a/tests/checkasm/aacencdsp.c b/tests/checkasm/aacencdsp.c
>>>>> index 684c775862..7cc8a01158 100644
>>>>> --- a/tests/checkasm/aacencdsp.c
>>>>> +++ b/tests/checkasm/aacencdsp.c
>>>>> @@ -19,6 +19,7 @@
>>>>>     */
>>>>>
>>>>>    #include <string.h>
>>>>> +#include <float.h>
>>>>>
>>>>>    #include "libavutil/mem.h"
>>>>>    #include "libavutil/mem_internal.h"
>>>>> @@ -51,7 +52,7 @@ static void test_abs_pow34(AACEncContext *s) {
>>>>>            call_ref(out, in, BUF_SIZE);
>>>>>            call_new(out2, in, BUF_SIZE);
>>>>>
>>>>> -        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
>>>>> +        if (!float_near_abs_eps_array(out, out2, FLT_EPSILON, BUF_SIZE))
>>>>>                fail();
>>>>>
>>>>>            bench_new(out, in, BUF_SIZE);
>>>>
>>>> That target tends to be pretty picky about the epsilon value because of the
>>>> 80 bits precision for floats.
>>>
>>>
>>> with float.h
>>>
>>> float_near_abs_eps_array
>>>
>>> Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
>>> checkasm: using random seed 4097080137
>>> test failed comparing 5.68635 with 5.68635 (abs diff=4.76837e-07 with EPS=1.19209e-07)
>>> SSE:
>>>      abs_pow34_sse (aacencdsp.c:56)
>>>    - aacencdsp.abs_pow34 [FAILED]
>>> checkasm: 1 of 1 tests have failed
>>> threads=1
>>> src/tests/Makefile:317: recipe for target 'fate-checkasm-aacencdsp' failed
>>> make: *** [fate-checkasm-aacencdsp] Error 1
>>>
>>> if (!float_near_abs_eps_array(out, out2, FLT_EPSILON*10, BUF_SIZE))
>>> works but feels wrong
>>>
>>> this works too (only tested 100x on only x86-32 yet)
>>> if (!float_near_ulp_array(out, out2, 1, BUF_SIZE))
>>>
>>> thx
>>
>> 1 is excessive. Some tests use an epsilon value like 0.005, so
>> FLT_EPSILON*10 is IMO fine. Or we could hardcode something like 0.000005
>> instead if it passes.
> 
> 1 ULP is the smallest that works
> tested 100x on mips, arm, mingw32 now too
> 
> epsilon anything is IMHO wrong but iam happy to hear an argument why
> absolute is better than relative here (which is the difference bascially
> IIUC)
> 
> absolute is bad as the difference in what can be represented is relative
> to the values size and so a absolute check will inevitably be very loose
> on small values or will fail on large ones

My bad, didn't notice you suggested float_near_ulp_array() instead of 
float_near_abs_eps_array(), and thought you were suggesting an epsilon 
of 1.0

Should be ok then.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34
  2024-01-23 19:52           ` James Almer
@ 2024-01-23 20:09             ` Michael Niedermayer
  0 siblings, 0 replies; 7+ messages in thread
From: Michael Niedermayer @ 2024-01-23 20:09 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 6278 bytes --]

On Tue, Jan 23, 2024 at 04:52:07PM -0300, James Almer wrote:
> On 1/23/2024 4:49 PM, Michael Niedermayer wrote:
> > On Tue, Jan 23, 2024 at 04:40:46PM -0300, James Almer wrote:
> > > On 1/23/2024 4:33 PM, Michael Niedermayer wrote:
> > > > On Tue, Jan 23, 2024 at 04:17:22PM -0300, James Almer wrote:
> > > > > On 1/23/2024 4:07 PM, Michael Niedermayer wrote:
> > > > > > On Mon, Dec 11, 2023 at 04:42:26PM +0000, sunyuechi wrote:
> > > > > > > ffmpeg | branch: master | sunyuechi <sunyuechi@iscas.ac.cn> | Tue Nov 28 14:08:12 2023 +0800| [1c3620b2bbe73db9239fcf605e8f535b58f03b86] | committer: Rémi Denis-Courmont
> > > > > > > 
> > > > > > > checkasm: test for abs_pow34
> > > > > > > 
> > > > > > > Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
> > > > > > > 
> > > > > > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=1c3620b2bbe73db9239fcf605e8f535b58f03b86
> > > > > > > ---
> > > > > > > 
> > > > > > >     tests/checkasm/Makefile    |  1 +
> > > > > > >     tests/checkasm/aacencdsp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >     tests/checkasm/checkasm.c  |  3 ++
> > > > > > >     tests/checkasm/checkasm.h  |  1 +
> > > > > > >     tests/fate/checkasm.mak    |  3 +-
> > > > > > >     5 files changed, 77 insertions(+), 1 deletion(-)
> > > > > > [...]
> > > > > > > +static void test_abs_pow34(AACEncContext *s) {
> > > > > > > +#define BUF_SIZE 1024
> > > > > > > +    LOCAL_ALIGNED_32(float, in, [BUF_SIZE]);
> > > > > > > +
> > > > > > > +    declare_func(void, float *, const float *, int);
> > > > > > > +
> > > > > > > +    randomize_float(in, BUF_SIZE);
> > > > > > > +
> > > > > > > +    if (check_func(s->abs_pow34, "abs_pow34")) {
> > > > > > > +        LOCAL_ALIGNED_32(float, out, [BUF_SIZE]);
> > > > > > > +        LOCAL_ALIGNED_32(float, out2, [BUF_SIZE]);
> > > > > > > +
> > > > > > > +        call_ref(out, in, BUF_SIZE);
> > > > > > > +        call_new(out2, in, BUF_SIZE);
> > > > > > > +
> > > > > > > +        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> > > > > > > +            fail();
> > > > > > 
> > > > > > This is wrong if one of the functions is implemented in C
> > > > > > C does not specify float to give bitexact results. Any equality check with
> > > > > > float thats expected to end in a reproduceable outcome is generally wrong
> > > > > 
> > > > > Yes, that's why float_near_abs_eps_array() or float_near_ulp_array() should
> > > > > be used.
> > > > > 
> > > > > > 
> > > > > > it also fails in practice
> > > > > > 
> > > > > > TEST    checkasm-aacencdsp
> > > > > > Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
> > > > > > src/tests/Makefile:308: recipe for target 'fate-checkasm-aacencdsp' failed
> > > > > > make: *** [fate-checkasm-aacencdsp] Error 1
> > > > > > 
> > > > > > checkasm: using random seed 2523748868
> > > > > > SSE:
> > > > > >       abs_pow34_sse (aacencdsp.c:55)
> > > > > >     - aacencdsp.abs_pow34 [FAILED]
> > > > > > checkasm: 1 of 1 tests have failed
> > > > > > threads=1
> > > > > > 
> > > > > > thats on plain simple x86-32
> > > > > 
> > > > > Can you check if the following passes for x86-32?
> > > > > 
> > > > > > diff --git a/tests/checkasm/aacencdsp.c b/tests/checkasm/aacencdsp.c
> > > > > > index 684c775862..7cc8a01158 100644
> > > > > > --- a/tests/checkasm/aacencdsp.c
> > > > > > +++ b/tests/checkasm/aacencdsp.c
> > > > > > @@ -19,6 +19,7 @@
> > > > > >     */
> > > > > > 
> > > > > >    #include <string.h>
> > > > > > +#include <float.h>
> > > > > > 
> > > > > >    #include "libavutil/mem.h"
> > > > > >    #include "libavutil/mem_internal.h"
> > > > > > @@ -51,7 +52,7 @@ static void test_abs_pow34(AACEncContext *s) {
> > > > > >            call_ref(out, in, BUF_SIZE);
> > > > > >            call_new(out2, in, BUF_SIZE);
> > > > > > 
> > > > > > -        if (memcmp(out, out2, BUF_SIZE * sizeof(float)) != 0)
> > > > > > +        if (!float_near_abs_eps_array(out, out2, FLT_EPSILON, BUF_SIZE))
> > > > > >                fail();
> > > > > > 
> > > > > >            bench_new(out, in, BUF_SIZE);
> > > > > 
> > > > > That target tends to be pretty picky about the epsilon value because of the
> > > > > 80 bits precision for floats.
> > > > 
> > > > 
> > > > with float.h
> > > > 
> > > > float_near_abs_eps_array
> > > > 
> > > > Test checkasm-aacencdsp failed. Look at tests/data/fate/checkasm-aacencdsp.err for details.
> > > > checkasm: using random seed 4097080137
> > > > test failed comparing 5.68635 with 5.68635 (abs diff=4.76837e-07 with EPS=1.19209e-07)
> > > > SSE:
> > > >      abs_pow34_sse (aacencdsp.c:56)
> > > >    - aacencdsp.abs_pow34 [FAILED]
> > > > checkasm: 1 of 1 tests have failed
> > > > threads=1
> > > > src/tests/Makefile:317: recipe for target 'fate-checkasm-aacencdsp' failed
> > > > make: *** [fate-checkasm-aacencdsp] Error 1
> > > > 
> > > > if (!float_near_abs_eps_array(out, out2, FLT_EPSILON*10, BUF_SIZE))
> > > > works but feels wrong
> > > > 
> > > > this works too (only tested 100x on only x86-32 yet)
> > > > if (!float_near_ulp_array(out, out2, 1, BUF_SIZE))
> > > > 
> > > > thx
> > > 
> > > 1 is excessive. Some tests use an epsilon value like 0.005, so
> > > FLT_EPSILON*10 is IMO fine. Or we could hardcode something like 0.000005
> > > instead if it passes.
> > 
> > 1 ULP is the smallest that works
> > tested 100x on mips, arm, mingw32 now too
> > 
> > epsilon anything is IMHO wrong but iam happy to hear an argument why
> > absolute is better than relative here (which is the difference bascially
> > IIUC)
> > 
> > absolute is bad as the difference in what can be represented is relative
> > to the values size and so a absolute check will inevitably be very loose
> > on small values or will fail on large ones
> 
> My bad, didn't notice you suggested float_near_ulp_array() instead of
> float_near_abs_eps_array(), and thought you were suggesting an epsilon of
> 1.0
> 
> Should be ok then.

will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-01-23 20:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20231211164227.163BB4115B0@natalya.videolan.org>
2024-01-23 19:07 ` [FFmpeg-devel] [FFmpeg-cvslog] checkasm: test for abs_pow34 Michael Niedermayer
2024-01-23 19:17   ` James Almer
2024-01-23 19:33     ` Michael Niedermayer
2024-01-23 19:40       ` James Almer
2024-01-23 19:49         ` Michael Niedermayer
2024-01-23 19:52           ` James Almer
2024-01-23 20:09             ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git