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

unit test fails on i386 #89

Open
josch opened this Issue Feb 1, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@josch
Copy link
Contributor

josch commented Feb 1, 2019

You can reproduce this issue by compiling fuzzylite inside a i386 chroot and then trying to run the tests. Here is a full error log (scroll all the way to the bottom to see the problem):

https://buildd.debian.org/status/fetch.php?pkg=fuzzylite&arch=i386&ver=6.0%2Bdfsg-1&stamp=1549055053&raw=0

According to this overview

https://buildd.debian.org/status/package.php?p=fuzzylite

The problem only shows on i386 and not on other 32bit architectures like armel or mipsel.

@josch

This comment has been minimized.

Copy link
Contributor Author

josch commented Feb 2, 2019

I tracked down the issue to the following 2-liner in C:

double convert(double a, double b) { return a/b; }
int main() { printf("%.15f\n", 35e9 - convert(35, 1.0e-9)); }

On i386, it prints:

0.000002179294825

On all other architectures (including other 32 bit architectures) we get:

0.000000000000000
@josch

This comment has been minimized.

Copy link
Contributor Author

josch commented Feb 2, 2019

The culprit might be the 80bit x87 register on i386 platforms. There are several ways forward that fix the problem:

  • compile with -ffloat-store which might make fuzzylite very slow (I didn't benchmark)
  • compile with -mfpmath=sse -msse2 which would require SSE and thus make fuzzylite incompatible with many i386 platforms
  • in fuzzylite/src/fuzzylite.cpp, increase fuzzylite::_macheps from 1e-6 to 1e-5 for i386 only.
  • in fuzzylite/test/BenchmarkTest.cpp, call Op::isEq with an additional argument setting the epsilon to 1e-5 for the 35e9 check on i386 only
@josch

This comment has been minimized.

Copy link
Contributor Author

josch commented Feb 3, 2019

In Debian I now solved the problem with this patch:

--- fuzzylite-6.0+dfsg.orig/fuzzylite/test/BenchmarkTest.cpp
+++ fuzzylite-6.0+dfsg/fuzzylite/test/BenchmarkTest.cpp
@@ -96,7 +96,17 @@ namespace fl {
         CHECK(Op::isEq(1.0, Benchmark::convert(1000.0, Benchmark::MilliSeconds, Benchmark::Seconds)));
         FL_LOG(Benchmark::convert(1000.0, Benchmark::MilliSeconds, Benchmark::Seconds));
 
-        CHECK(Op::isEq(35e9, Benchmark::convert(35, Benchmark::Seconds, Benchmark::NanoSeconds)));
+        scalar eps =
+#ifndef __i386__
+            fuzzylite::macheps();
+#else
+            // on i386, due to the 80bit x87 register, double floating point
+            // numbers are handled differently and thus the difference between
+            // 35e9 and the result of Benchmark::convert() will be 2.179e-6,
+            // which is greater than the default epsilon of 1e-6.
+            1e-5;
+#endif
+        CHECK(Op::isEq(35e9, Benchmark::convert(35, Benchmark::Seconds, Benchmark::NanoSeconds), eps));
         CHECK(Op::isEq(35, Benchmark::convert(35e9, Benchmark::NanoSeconds, Benchmark::Seconds)));
     }
@jcrada

This comment has been minimized.

Copy link
Member

jcrada commented Feb 6, 2019

Hi josch. Many thanks for your help. I think it is better to just change the default value to 1e-5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment