Bug#887323: dewalls i386 test failure
On 2018-03-09 19:24 +, Philip Schuchardt wrote: > Wow, we were using long doubles before. I don't think we need that much > precision for cave survey. In catch there is Approx() that should take care of > floating point comparison correctly. Yes. Catch has approx and margin and epsilon which let us specify accuracy/precision for the tests, but don't solve the throw/no-throw issue because that's a binary test on the catch side - all the thresholds are entirely on the dewalls side. In the meantime the bodge in 1.0.0+ds1-5 has fixed the builds on all arches: https://buildd.debian.org/status/package.php?p=dewalls Wookey -- Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/ signature.asc Description: PGP signature
Bug#887323: dewalls i386 test failure
So we need to fixed the comparisons on the dewalls library.
On Fri, Mar 9, 2018 at 2:24 PM Philip Schuchardt wrote:
> Wow, we were using long doubles before. I don't think we need that much
> precision for cave survey. In catch there is Approx() that should take care
> of floating point comparison correctly.
>
> On Fri, Mar 9, 2018 at 11:30 AM Wookey wrote:
>
>> A bit of research tells that using 'long double' causes 80-bit fp on x86
>> (i386 and amd64), as opposed to the 64-bit IEE 754 fp representation used
>> on other platforms, which is what you get if you specify double.
>>
>> So I tried changing the angle.{cpp.h} code to use double instead of long
>> double.
>>
>> That doesn't fix the problrematic tests, but does cause another 43 to
>> fail:
>> test cases: 8 | 3 passed | 5 failed
>> assertions: 202 | 155 passed | 47 failed
>>
>> They all fail equality tests, despite them clearly being 'equal
>> enough' for our purposes.
>>
>> e.g.
>> /home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:31: FAILED:
>> CHECK( WallsSurveyParser("5:4").azimuth(Angle::Degrees) == UAngle(5 + 4
>> / 60.0, Angle::Degrees) )
>> with expansion:
>> 5.06667 deg == 5.06667 deg
>>
>> I see that 'approx' is used in the codebase which you might hope would
>> get all this right, but clearly excessive precision is being used for
>> our purposes (cave survey data).
>>
>> I'm not sure how to fix this.
>>
>> I've done an upload in the meantime with the previous bodge of the
>> tests, and to fix another bug too and let the package migrate to
>> testing. But getting the codebase to do what we actually expect would
>> be good.
>>
>> This is the patch I used to try double for angles:
>>
>> Author: Wookey
>> Last-Update: 2018-03-09
>>
>> --- dewalls-1.0.0+ds1.orig/src/angle.cpp
>> +++ dewalls-1.0.0+ds1/src/angle.cpp
>> @@ -3,13 +3,13 @@
>>
>> namespace dewalls {
>>
>> -const long double PI = acosl(-1.0L);
>> -const long double DegreesToRadians = PI / 180.0L;
>> -const long double GradiansToRadians = PI / 200.0L;
>> -const long double MilsNATOToRadians = PI / 3200.0L;
>> -const long double RadiansToDegrees = 180.0L / PI;
>> -const long double RadiansToGradians = 200.0L / PI;
>> -const long double RadiansToMilsNATO = 3200.0L / PI;
>> +const double PI = acosl(-1.0L);
>> +const double DegreesToRadians = PI / 180.0L;
>> +const double GradiansToRadians = PI / 200.0L;
>> +const double MilsNATOToRadians = PI / 3200.0L;
>> +const double RadiansToDegrees = 180.0L / PI;
>> +const double RadiansToGradians = 200.0L / PI;
>> +const double RadiansToMilsNATO = 3200.0L / PI;
>>
>> QString Angle::Name("angle");
>>
>> @@ -30,7 +30,7 @@ QString Angle::symbolFor(Unit unit) {
>> }
>> }
>>
>> -long double Angle::toBase(long double quantity, Unit fromUnit) {
>> +double Angle::toBase(double quantity, Unit fromUnit) {
>> switch (fromUnit) {
>> case Radians:
>> return quantity;
>> @@ -47,7 +47,7 @@ long double Angle::toBase(long double qu
>> }
>> }
>>
>> -long double Angle::fromBase(long double quantity, Unit toUnit) {
>> +double Angle::fromBase(double quantity, Unit toUnit) {
>> switch (toUnit) {
>> case Radians:
>> return quantity;
>> @@ -64,7 +64,7 @@ long double Angle::fromBase(long double
>> }
>> }
>>
>> -long double Angle::convert(long double quantity, Unit fromUnit, Unit
>> toUnit) {
>> +double Angle::convert(double quantity, Unit fromUnit, Unit toUnit) {
>> return fromBase(toBase(quantity, fromUnit), toUnit);
>> }
>>
>> --- dewalls-1.0.0+ds1.orig/src/angle.h
>> +++ dewalls-1.0.0+ds1/src/angle.h
>> @@ -21,9 +21,9 @@ public:
>>
>> static QString Name;
>>
>> -static long double toBase(long double quantity, Unit fromUnit);
>> -static long double fromBase(long double quantity, Unit toUnit);
>> -static long double convert(long double quantity, Unit fromUnit, Unit
>> toUnit);
>> +static double toBase(double quantity, Unit fromUnit);
>> +static double fromBase(double quantity, Unit toUnit);
>> +static double convert(double quantity, Unit fromUnit, Unit toUnit);
>> static QString symbolFor(Unit unit);
>> };
>>
>>
>>
>> Wookey
>> --
>> Principal hats: Linaro, Debian, Wookware, ARM
>> http://wookware.org/
>>
> --
> Phi|ip
>
--
Phi|ip
Bug#887323: dewalls i386 test failure
Wow, we were using long doubles before. I don't think we need that much
precision for cave survey. In catch there is Approx() that should take care
of floating point comparison correctly.
On Fri, Mar 9, 2018 at 11:30 AM Wookey wrote:
> A bit of research tells that using 'long double' causes 80-bit fp on x86
> (i386 and amd64), as opposed to the 64-bit IEE 754 fp representation used
> on other platforms, which is what you get if you specify double.
>
> So I tried changing the angle.{cpp.h} code to use double instead of long
> double.
>
> That doesn't fix the problrematic tests, but does cause another 43 to fail:
> test cases: 8 | 3 passed | 5 failed
> assertions: 202 | 155 passed | 47 failed
>
> They all fail equality tests, despite them clearly being 'equal
> enough' for our purposes.
>
> e.g.
> /home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:31: FAILED:
> CHECK( WallsSurveyParser("5:4").azimuth(Angle::Degrees) == UAngle(5 + 4
> / 60.0, Angle::Degrees) )
> with expansion:
> 5.06667 deg == 5.06667 deg
>
> I see that 'approx' is used in the codebase which you might hope would
> get all this right, but clearly excessive precision is being used for
> our purposes (cave survey data).
>
> I'm not sure how to fix this.
>
> I've done an upload in the meantime with the previous bodge of the
> tests, and to fix another bug too and let the package migrate to
> testing. But getting the codebase to do what we actually expect would
> be good.
>
> This is the patch I used to try double for angles:
>
> Author: Wookey
> Last-Update: 2018-03-09
>
> --- dewalls-1.0.0+ds1.orig/src/angle.cpp
> +++ dewalls-1.0.0+ds1/src/angle.cpp
> @@ -3,13 +3,13 @@
>
> namespace dewalls {
>
> -const long double PI = acosl(-1.0L);
> -const long double DegreesToRadians = PI / 180.0L;
> -const long double GradiansToRadians = PI / 200.0L;
> -const long double MilsNATOToRadians = PI / 3200.0L;
> -const long double RadiansToDegrees = 180.0L / PI;
> -const long double RadiansToGradians = 200.0L / PI;
> -const long double RadiansToMilsNATO = 3200.0L / PI;
> +const double PI = acosl(-1.0L);
> +const double DegreesToRadians = PI / 180.0L;
> +const double GradiansToRadians = PI / 200.0L;
> +const double MilsNATOToRadians = PI / 3200.0L;
> +const double RadiansToDegrees = 180.0L / PI;
> +const double RadiansToGradians = 200.0L / PI;
> +const double RadiansToMilsNATO = 3200.0L / PI;
>
> QString Angle::Name("angle");
>
> @@ -30,7 +30,7 @@ QString Angle::symbolFor(Unit unit) {
> }
> }
>
> -long double Angle::toBase(long double quantity, Unit fromUnit) {
> +double Angle::toBase(double quantity, Unit fromUnit) {
> switch (fromUnit) {
> case Radians:
> return quantity;
> @@ -47,7 +47,7 @@ long double Angle::toBase(long double qu
> }
> }
>
> -long double Angle::fromBase(long double quantity, Unit toUnit) {
> +double Angle::fromBase(double quantity, Unit toUnit) {
> switch (toUnit) {
> case Radians:
> return quantity;
> @@ -64,7 +64,7 @@ long double Angle::fromBase(long double
> }
> }
>
> -long double Angle::convert(long double quantity, Unit fromUnit, Unit
> toUnit) {
> +double Angle::convert(double quantity, Unit fromUnit, Unit toUnit) {
> return fromBase(toBase(quantity, fromUnit), toUnit);
> }
>
> --- dewalls-1.0.0+ds1.orig/src/angle.h
> +++ dewalls-1.0.0+ds1/src/angle.h
> @@ -21,9 +21,9 @@ public:
>
> static QString Name;
>
> -static long double toBase(long double quantity, Unit fromUnit);
> -static long double fromBase(long double quantity, Unit toUnit);
> -static long double convert(long double quantity, Unit fromUnit, Unit
> toUnit);
> +static double toBase(double quantity, Unit fromUnit);
> +static double fromBase(double quantity, Unit toUnit);
> +static double convert(double quantity, Unit fromUnit, Unit toUnit);
> static QString symbolFor(Unit unit);
> };
>
>
>
> Wookey
> --
> Principal hats: Linaro, Debian, Wookware, ARM
> http://wookware.org/
>
--
Phi|ip
Bug#887323: dewalls i386 test failure
A bit of research tells that using 'long double' causes 80-bit fp on x86 (i386
and amd64), as opposed to the 64-bit IEE 754 fp representation used on other
platforms, which is what you get if you specify double.
So I tried changing the angle.{cpp.h} code to use double instead of long double.
That doesn't fix the problrematic tests, but does cause another 43 to fail:
test cases: 8 | 3 passed | 5 failed
assertions: 202 | 155 passed | 47 failed
They all fail equality tests, despite them clearly being 'equal
enough' for our purposes.
e.g.
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:31: FAILED:
CHECK( WallsSurveyParser("5:4").azimuth(Angle::Degrees) == UAngle(5 + 4 /
60.0, Angle::Degrees) )
with expansion:
5.06667 deg == 5.06667 deg
I see that 'approx' is used in the codebase which you might hope would
get all this right, but clearly excessive precision is being used for
our purposes (cave survey data).
I'm not sure how to fix this.
I've done an upload in the meantime with the previous bodge of the
tests, and to fix another bug too and let the package migrate to
testing. But getting the codebase to do what we actually expect would
be good.
This is the patch I used to try double for angles:
Author: Wookey
Last-Update: 2018-03-09
--- dewalls-1.0.0+ds1.orig/src/angle.cpp
+++ dewalls-1.0.0+ds1/src/angle.cpp
@@ -3,13 +3,13 @@
namespace dewalls {
-const long double PI = acosl(-1.0L);
-const long double DegreesToRadians = PI / 180.0L;
-const long double GradiansToRadians = PI / 200.0L;
-const long double MilsNATOToRadians = PI / 3200.0L;
-const long double RadiansToDegrees = 180.0L / PI;
-const long double RadiansToGradians = 200.0L / PI;
-const long double RadiansToMilsNATO = 3200.0L / PI;
+const double PI = acosl(-1.0L);
+const double DegreesToRadians = PI / 180.0L;
+const double GradiansToRadians = PI / 200.0L;
+const double MilsNATOToRadians = PI / 3200.0L;
+const double RadiansToDegrees = 180.0L / PI;
+const double RadiansToGradians = 200.0L / PI;
+const double RadiansToMilsNATO = 3200.0L / PI;
QString Angle::Name("angle");
@@ -30,7 +30,7 @@ QString Angle::symbolFor(Unit unit) {
}
}
-long double Angle::toBase(long double quantity, Unit fromUnit) {
+double Angle::toBase(double quantity, Unit fromUnit) {
switch (fromUnit) {
case Radians:
return quantity;
@@ -47,7 +47,7 @@ long double Angle::toBase(long double qu
}
}
-long double Angle::fromBase(long double quantity, Unit toUnit) {
+double Angle::fromBase(double quantity, Unit toUnit) {
switch (toUnit) {
case Radians:
return quantity;
@@ -64,7 +64,7 @@ long double Angle::fromBase(long double
}
}
-long double Angle::convert(long double quantity, Unit fromUnit, Unit toUnit) {
+double Angle::convert(double quantity, Unit fromUnit, Unit toUnit) {
return fromBase(toBase(quantity, fromUnit), toUnit);
}
--- dewalls-1.0.0+ds1.orig/src/angle.h
+++ dewalls-1.0.0+ds1/src/angle.h
@@ -21,9 +21,9 @@ public:
static QString Name;
-static long double toBase(long double quantity, Unit fromUnit);
-static long double fromBase(long double quantity, Unit toUnit);
-static long double convert(long double quantity, Unit fromUnit, Unit
toUnit);
+static double toBase(double quantity, Unit fromUnit);
+static double fromBase(double quantity, Unit toUnit);
+static double convert(double quantity, Unit fromUnit, Unit toUnit);
static QString symbolFor(Unit unit);
};
Wookey
--
Principal hats: Linaro, Debian, Wookware, ARM
http://wookware.org/
signature.asc
Description: PGP signature
Bug#887323: dewalls i386 test failure
On 2018-03-09 05:06 +, Wookey wrote:
>
> Changing the tests to this:
> CHECK_THROWS( WallsSurveyParser("360.1").azimuth(Angle::Degrees)
> );
> CHECK_THROWS( WallsSurveyParser("-0.1").azimuth(Angle::Degrees) );
> CHECK_THROWS(
> WallsSurveyParser("400.1g").azimuth(Angle::Gradians) );
> CHECK_THROWS( WallsSurveyParser("-0.1g").azimuth(Angle::Gradians)
> );
> CHECK_THROWS( WallsSurveyParser("N90.1E").azimuth(Angle::Degrees)
> );
> CHECK_THROWS(
> WallsSurveyParser("N100.1gE").azimuth(Angle::Gradians) );
>
> makes them all pass. Which strongly suggsts that this really is a
> rounding problem on i386. I could experiment further to try and
> determine the size of the rounding error.
One more zero and we are back to failing:
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:64: FAILED:
CHECK_THROWS( WallsSurveyParser("360.01").azimuth(Angle::Degrees) )
because no exception was thrown where one was expected:
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:66: FAILED:
CHECK_THROWS( WallsSurveyParser("400.01g").azimuth(Angle::Gradians) )
because no exception was thrown where one was expected:
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:68: FAILED:
CHECK_THROWS( WallsSurveyParser("N90.01E").azimuth(Angle::Degrees) )
because no exception was thrown where one was expected:
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:69: FAILED:
CHECK_THROWS( WallsSurveyParser("N100.01gE").azimuth(Angle::Gradians) )
because no exception was thrown where one was expected:
===
test cases:8 |7 passed | 1 failed
assertions: 2729 | 2725 passed | 4 failed
I see that catch provides mechanisms for allowing for epsilon in
floating-point comparisons:
https://github.com/catchorg/Catch2/blob/master/docs/assertions.md#natural-expressions
however it's not catch doing the comparison here so that doesn't
help. dewalls needs to make the allowance. Strictly speaking this
shouldn't be an f.p. test for 'value too high' - it should be a string
test for 'inadmissible number' - i.e the cutoff is absolute and not
subject to epsilon.
I'm going to leave the test value adjusted so that the throw actually
triggers for now, as a functioning workaround.
Wookey
--
Principal hats: Linaro, Debian, Wookware, ARM
http://wookware.org/
signature.asc
Description: PGP signature
Bug#887323: dewalls i386 test failure
Is this test actually right? Shouldn't it be Angle::Gradians?
CHECK_THROWS( WallsSurveyParser("400g").azimuth(Angle::Degrees) )
same for;
CHECK_THROWS( WallsSurveyParser("-0.1g").azimuth(Angle::Degrees) );
CHECK_THROWS( WallsSurveyParser("N100gE").azimuth(Angle::Degrees) );
I tried changing the test to:
CHECK_THROWS( WallsSurveyParser("400g").azimuth(Angle::Gradians) )
CHECK_THROWS( WallsSurveyParser("400").azimuth(Angle::Gradians) )
but they both still fail.
Changing the tests to this:
CHECK_THROWS( WallsSurveyParser("360.1").azimuth(Angle::Degrees) );
CHECK_THROWS( WallsSurveyParser("-0.1").azimuth(Angle::Degrees) );
CHECK_THROWS( WallsSurveyParser("400.1g").azimuth(Angle::Gradians)
);
CHECK_THROWS( WallsSurveyParser("-0.1g").azimuth(Angle::Gradians) );
CHECK_THROWS( WallsSurveyParser("N90.1E").azimuth(Angle::Degrees) );
CHECK_THROWS(
WallsSurveyParser("N100.1gE").azimuth(Angle::Gradians) );
makes them all pass. Which strongly suggsts that this really is a
rounding problem on i386. I could experiment further to try and
determine the size of the rounding error.
I could leave this bodge in in order to get i386 built and dewalls
migrated into testing, but it would be better to actually fix the
problem if possible. I could also arrange not to run the tests on
i386.
Wookey
--
Principal hats: Linaro, Debian, Wookware, ARM
http://wookware.org/
signature.asc
Description: PGP signature
Bug#887323: dewalls i386 test failure
On 2018-01-31 20:59 +, Philip Schuchardt wrote:
> Nice! I wonder if i386 don’t support exceptions properly or something...
It's not that simple.
I found out how to run specific tests with catch. There are two other groups of
tests which are expected to throw and those work OK:
(sid_i386-dchroot)wookey@barriere:~/dewalls-1.0.0+ds1$
qbs-build/dewalls-test.a3cdf9ea/dewalls-test -f test/azimuthparsingtests.cpp
azimuth* -c "misc invalid values throw"
===
All tests passed (6 assertions in 1 test case)
(sid_i386-dchroot)wookey@barriere:~/dewalls-1.0.0+ds1$
qbs-build/dewalls-test.a3cdf9ea/dewalls-test -f test/azimuthparsingtests.cpp
azimuth* -c "negative numbers throw for azimuth"
===
All tests passed (1 assertion in 1 test case)
But the 'out of range values throw' tests fail:
(sid_i386-dchroot)wookey@barriere:~/dewalls-1.0.0+ds1$
qbs-build/dewalls-test.a3cdf9ea/dewalls-test -f test/azimuthparsingtests.cpp
azimuth* -c "out of range values throw"
~~~
dewalls-test is a Catch v1.10.0 host application.
Run with -? for options
---
azimuth parsing tests
out of range values throw
---
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:63
...
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:64: FAILED:
CHECK_THROWS( WallsSurveyParser("360").azimuth(Angle::Degrees) )
because no exception was thrown where one was expected:
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:66: FAILED:
CHECK_THROWS( WallsSurveyParser("400g").azimuth(Angle::Degrees) )
because no exception was thrown where one was expected:
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:68: FAILED:
CHECK_THROWS( WallsSurveyParser("N90E").azimuth(Angle::Degrees) )
because no exception was thrown where one was expected:
/home/wookey/dewalls-1.0.0+ds1/test/azimuthparsingtests.cpp:69: FAILED:
CHECK_THROWS( WallsSurveyParser("N100gE").azimuth(Angle::Degrees) )
because no exception was thrown where one was expected:
===
test cases: 1 | 1 failed
assertions: 6 | 2 passed | 4 failed
I'm not sure what to make of this, but it suggests that the issue is
more likely to be not with catch itself, but with the actual parser.
How can I test the parser directly, to eliminate catch as part of the issue?
Can one of you knock up a little test case that tries to read in a
file containing these sorts of error, that can be built against
libdewalls? Then we can see whether the library is actually getting
this wrong or if it's only in the test framework.
Wookey
--
Principal hats: Linaro, Debian, Wookware, ARM
http://wookware.org/
signature.asc
Description: PGP signature
Bug#887323: dewalls i386 test failure
Nice! I wonder if i386 don’t support exceptions properly or something... On Tue, Jan 30, 2018 at 8:07 AM Wookey wrote: > On 2018-01-30 06:48 +, Philip Schuchardt wrote: > > I know I’ve been dragging my feet on this. Let’s see if I can figure > this out, > > this week. > > I fished out an old netbook which is actually i386. And installed an > unstable chroot on it (very slowly!) last night. Hopefully this will > enable some bottom-getting-to. > > I should probbaly just have used a debian porter-box, but that machine > needed updating anyway. > > Wookey > -- > Principal hats: Linaro, Debian, Wookware, ARM > http://wookware.org/ > -- Phi|ip
Bug#887323: dewalls i386 test failure
On 2018-01-30 06:48 +, Philip Schuchardt wrote: > I know I’ve been dragging my feet on this. Let’s see if I can figure this out, > this week. I fished out an old netbook which is actually i386. And installed an unstable chroot on it (very slowly!) last night. Hopefully this will enable some bottom-getting-to. I should probbaly just have used a debian porter-box, but that machine needed updating anyway. Wookey -- Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/ signature.asc Description: PGP signature
