[v1] test: Work around x87 floating point comparison issues
diff mbox series

Message ID 20261012110635.482661-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v1] test: Work around x87 floating point comparison issues
Related show

Commit Message

Barnabás Pőcze Oct. 12, 2026, 11:06 a.m. UTC
A few tests compare floating point values using the built-in equality
operators. This is not ideal at all, but bringing in nuance filled floating
point comparison does not appear too appealing either, especially just for a
couple tests.

So instead work around these comparison failures by using numbers that are
exactly representable in base 2, and which ensure that the operations will be
exact as well. This is of course far from the best approach, but seems
acceptable given that the changes are very simple and only tests are affected
in the first place.

Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/348
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 test/controls/control_value.cpp | 8 +++++---
 test/ipa/libipa/histogram.cpp   | 6 +++---
 2 files changed, 8 insertions(+), 6 deletions(-)

Comments

Kieran Bingham Oct. 12, 2026, 11:21 a.m. UTC | #1
Quoting Barnabás Pőcze (2026-10-12 12:06:35)
> A few tests compare floating point values using the built-in equality
> operators. This is not ideal at all, but bringing in nuance filled floating
> point comparison does not appear too appealing either, especially just for a
> couple tests.
> 
> So instead work around these comparison failures by using numbers that are
> exactly representable in base 2, and which ensure that the operations will be
> exact as well. This is of course far from the best approach, but seems
> acceptable given that the changes are very simple and only tests are affected
> in the first place.
> 
> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/348
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Seems like at least a clean workaround.

I don't think it breaks any assumptions of what was being tested either
so:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  test/controls/control_value.cpp | 8 +++++---
>  test/ipa/libipa/histogram.cpp   | 6 +++---
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> index c2f9fc4826..399f4b2bb1 100644
> --- a/test/controls/control_value.cpp
> +++ b/test/controls/control_value.cpp
> @@ -272,19 +272,21 @@ protected:
>                 /*
>                  * Float type.
>                  */
> -               value.set(-0.42f);
> +               constexpr float fValue = -0.75;
> +
> +               value.set(fValue);
>                 if (value.isNone() || value.isArray() ||
>                     value.type() != ControlTypeFloat) {
>                         cerr << "Control type mismatch after setting to float" << endl;
>                         return TestFail;
>                 }
>  
> -               if (value.get<float>() != -0.42f) {
> +               if (value.get<float>() != fValue) {
>                         cerr << "Control value mismatch after setting to float" << endl;
>                         return TestFail;
>                 }
>  
> -               if (value.toString() != "-0.420000") {
> +               if (value.toString() != "-0.750000") {
>                         cerr << "Control string mismatch after setting to float" << endl;
>                         return TestFail;
>                 }
> diff --git a/test/ipa/libipa/histogram.cpp b/test/ipa/libipa/histogram.cpp
> index 77ff31a6db..504c9ed319 100644
> --- a/test/ipa/libipa/histogram.cpp
> +++ b/test/ipa/libipa/histogram.cpp
> @@ -46,7 +46,7 @@ protected:
>                 ASSERT_EQ(hist.quantile(0.75), 1.5);
>  
>                 /* Test quantile smaller than the smallest histogram step. */
> -               ASSERT_EQ(hist.quantile(0.001), 0.002);
> +               ASSERT_EQ(hist.quantile(0.125), 0.25);
>  
>                 ASSERT_EQ(hist.interQuantileMean(0.0, 1.0), 1.0);
>                 ASSERT_EQ(hist.interQuantileMean(0.0, 0.5), 0.5);
> @@ -56,8 +56,8 @@ protected:
>                 ASSERT_EQ(hist.interQuantileMean(0.25, 0.75), 1.0);
>  
>                 /* Test small ranges at the borders of the histogram. */
> -               ASSERT_EQ(hist.interQuantileMean(0.0, 0.1), 0.1);
> -               ASSERT_EQ(hist.interQuantileMean(0.9, 1.0), 1.9);
> +               ASSERT_EQ(hist.interQuantileMean(0.0, 0.125), 0.125);
> +               ASSERT_EQ(hist.interQuantileMean(0.875, 1.0), 1.875);
>  
>                 return TestPass;
>         }
> -- 
> 2.56.0
>

Patch
diff mbox series

diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
index c2f9fc4826..399f4b2bb1 100644
--- a/test/controls/control_value.cpp
+++ b/test/controls/control_value.cpp
@@ -272,19 +272,21 @@  protected:
 		/*
 		 * Float type.
 		 */
-		value.set(-0.42f);
+		constexpr float fValue = -0.75;
+
+		value.set(fValue);
 		if (value.isNone() || value.isArray() ||
 		    value.type() != ControlTypeFloat) {
 			cerr << "Control type mismatch after setting to float" << endl;
 			return TestFail;
 		}
 
-		if (value.get<float>() != -0.42f) {
+		if (value.get<float>() != fValue) {
 			cerr << "Control value mismatch after setting to float" << endl;
 			return TestFail;
 		}
 
-		if (value.toString() != "-0.420000") {
+		if (value.toString() != "-0.750000") {
 			cerr << "Control string mismatch after setting to float" << endl;
 			return TestFail;
 		}
diff --git a/test/ipa/libipa/histogram.cpp b/test/ipa/libipa/histogram.cpp
index 77ff31a6db..504c9ed319 100644
--- a/test/ipa/libipa/histogram.cpp
+++ b/test/ipa/libipa/histogram.cpp
@@ -46,7 +46,7 @@  protected:
 		ASSERT_EQ(hist.quantile(0.75), 1.5);
 
 		/* Test quantile smaller than the smallest histogram step. */
-		ASSERT_EQ(hist.quantile(0.001), 0.002);
+		ASSERT_EQ(hist.quantile(0.125), 0.25);
 
 		ASSERT_EQ(hist.interQuantileMean(0.0, 1.0), 1.0);
 		ASSERT_EQ(hist.interQuantileMean(0.0, 0.5), 0.5);
@@ -56,8 +56,8 @@  protected:
 		ASSERT_EQ(hist.interQuantileMean(0.25, 0.75), 1.0);
 
 		/* Test small ranges at the borders of the histogram. */
-		ASSERT_EQ(hist.interQuantileMean(0.0, 0.1), 0.1);
-		ASSERT_EQ(hist.interQuantileMean(0.9, 1.0), 1.9);
+		ASSERT_EQ(hist.interQuantileMean(0.0, 0.125), 0.125);
+		ASSERT_EQ(hist.interQuantileMean(0.875, 1.0), 1.875);
 
 		return TestPass;
 	}