Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 13/01/2019 18:24, James Almer wrote: > I thought i had amended the patch, but guess not... > > I can revert and reapply with the amended commit message if you want, > but it will kinda litter the tree for not a lot of gain. Then again, the > tree is already a mess with all the merges. Not worth it, no. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 1/13/2019 3:18 PM, Derek Buitenhuis wrote: > On 13/01/2019 18:01, James Almer wrote: >> Pushes as is then. Thanks. > > Er. > > You didn't add the actual description of the problem/fix > to the commit message. I thought i had amended the patch, but guess not... I can revert and reapply with the amended commit message if you want, but it will kinda litter the tree for not a lot of gain. Then again, the tree is already a mess with all the merges. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 13/01/2019 18:01, James Almer wrote: > Pushes as is then. Thanks. Er. You didn't add the actual description of the problem/fix to the commit message. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 1/13/2019 12:55 PM, Derek Buitenhuis wrote: > On 13/01/2019 15:45, James Almer wrote: >> If there is, i don't know it. >> >> Float based fate tests have been fine tuned before when different >> architectures proved a certain stddev value was not lax enough, so this >> one could be increased if needed as well, but if you prefer i can use a >> big enough multiple of FLT_EPSILON instead. > > Don't really have a strong opinion on it, no. > > - Derek Pushes as is then. Thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 13/01/2019 15:45, James Almer wrote: > If there is, i don't know it. > > Float based fate tests have been fine tuned before when different > architectures proved a certain stddev value was not lax enough, so this > one could be increased if needed as well, but if you prefer i can use a > big enough multiple of FLT_EPSILON instead. Don't really have a strong opinion on it, no. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 1/13/2019 11:32 AM, Derek Buitenhuis wrote: > On 13/01/2019 02:44, James Almer wrote: >> Michael suggested 1000*FLT_EPSILON but IMO that's too big and may hide >> errors in future implementations. >> The value i used is the smallest value i found that didn't fail after >> several runs. 6.1e-05 for example fails. > > I figured that's how it was chosen (though not documented). I thought there > be a way to calculate it properly instead of empirically. If there is, i don't know it. Float based fate tests have been fine tuned before when different architectures proved a certain stddev value was not lax enough, so this one could be increased if needed as well, but if you prefer i can use a big enough multiple of FLT_EPSILON instead. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 13/01/2019 02:44, James Almer wrote: > Michael suggested 1000*FLT_EPSILON but IMO that's too big and may hide > errors in future implementations. > The value i used is the smallest value i found that didn't fail after > several runs. 6.1e-05 for example fails. I figured that's how it was chosen (though not documented). I thought there be a way to calculate it properly instead of empirically. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 1/11/2019 12:17 PM, Derek Buitenhuis wrote: > On 11/01/2019 13:28, Hendrik Leppkes wrote: >> Because the computation accumulates more inaccuarcy then FLT_EPSILON >> allows for. That value is really not of that great use. If you have >> two accurate numbers and do one calculation, it may work, but if you >> do a whole bunch of them, the error accumulates and eventually gets >> bigger then FLT_EPSILON. >> x86_32 floating point is for $reasons a tad bit less accurate then on >> x86_64, for example, resulting in the test failing. We have some other >> float tests that do (or used to) fail sporadically due to inaccuracy >> problems, which sometimes where fixed by similar means - or >> multiplifying FLT_EPSILON to make it bigger. > > OK. > > Two things: > > 1. That should be in the commit messages. > 2. Would some multiple of FLT_EPSILON make more sense? Michael suggested 1000*FLT_EPSILON but IMO that's too big and may hide errors in future implementations. The value i used is the smallest value i found that didn't fail after several runs. 6.1e-05 for example fails. > > - Derek > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 11/01/2019 13:28, Hendrik Leppkes wrote: > Because the computation accumulates more inaccuarcy then FLT_EPSILON > allows for. That value is really not of that great use. If you have > two accurate numbers and do one calculation, it may work, but if you > do a whole bunch of them, the error accumulates and eventually gets > bigger then FLT_EPSILON. > x86_32 floating point is for $reasons a tad bit less accurate then on > x86_64, for example, resulting in the test failing. We have some other > float tests that do (or used to) fail sporadically due to inaccuracy > problems, which sometimes where fixed by similar means - or > multiplifying FLT_EPSILON to make it bigger. OK. Two things: 1. That should be in the commit messages. 2. Would some multiple of FLT_EPSILON make more sense? - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On Fri, Jan 11, 2019 at 2:12 PM Derek Buitenhuis
wrote:
>
> On 10/01/2019 23:34, James Almer wrote:
> > -if (!float_near_abs_eps(cdst[i], odst[i], FLT_EPSILON)) {
> > +if (!float_near_abs_eps(cdst[i], odst[i], 6.2e-05)) {
>
> Can you elaborate a bit more on this? FLT_EPSILON is used correctly here
> as far as I can tell, and it is not clear why it fails on x86_32, and why
> we should choose an arbitrary unportable number instead (who knows if it
> explodes on weird systems).
>
Because the computation accumulates more inaccuarcy then FLT_EPSILON
allows for. That value is really not of that great use. If you have
two accurate numbers and do one calculation, it may work, but if you
do a whole bunch of them, the error accumulates and eventually gets
bigger then FLT_EPSILON.
x86_32 floating point is for $reasons a tad bit less accurate then on
x86_64, for example, resulting in the test failing. We have some other
float tests that do (or used to) fail sporadically due to inaccuracy
problems, which sometimes where fixed by similar means - or
multiplifying FLT_EPSILON to make it bigger.
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
On 10/01/2019 23:34, James Almer wrote:
> -if (!float_near_abs_eps(cdst[i], odst[i], FLT_EPSILON)) {
> +if (!float_near_abs_eps(cdst[i], odst[i], 6.2e-05)) {
Can you elaborate a bit more on this? FLT_EPSILON is used correctly here
as far as I can tell, and it is not clear why it fails on x86_32, and why
we should choose an arbitrary unportable number instead (who knows if it
explodes on weird systems).
- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference
Should fix failures on x86_32.
Signed-off-by: James Almer
---
tests/checkasm/af_afir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/checkasm/af_afir.c b/tests/checkasm/af_afir.c
index 54e2f68d6c..e3fb76e8e0 100644
--- a/tests/checkasm/af_afir.c
+++ b/tests/checkasm/af_afir.c
@@ -53,7 +53,7 @@ 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], FLT_EPSILON)) {
+if (!float_near_abs_eps(cdst[i], odst[i], 6.2e-05)) {
fprintf(stderr, "%d: %- .12f - %- .12f = % .12g\n",
i, cdst[i], odst[i], cdst[i] - odst[i]);
fail();
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
