[1/5] test: ipa: libipa: Add histogram tests
diff mbox series

Message ID 20250324170803.103296-2-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • Fix histogram for some (corner) cases
Related show

Commit Message

Stefan Klug March 24, 2025, 5:07 p.m. UTC 
Add some basic tests for the histogram class.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 test/ipa/libipa/histogram.cpp | 53 +++++++++++++++++++++++++++++++++++
 test/ipa/libipa/meson.build   |  1 +
 2 files changed, 54 insertions(+)
 create mode 100644 test/ipa/libipa/histogram.cpp

Comments

Kieran Bingham March 31, 2025, 6:03 p.m. UTC | #1 
Quoting Stefan Klug (2025-03-24 17:07:36)
> Add some basic tests for the histogram class.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  test/ipa/libipa/histogram.cpp | 53 +++++++++++++++++++++++++++++++++++
>  test/ipa/libipa/meson.build   |  1 +
>  2 files changed, 54 insertions(+)
>  create mode 100644 test/ipa/libipa/histogram.cpp
> 
> diff --git a/test/ipa/libipa/histogram.cpp b/test/ipa/libipa/histogram.cpp
> new file mode 100644
> index 000000000000..220a329e33b3
> --- /dev/null
> +++ b/test/ipa/libipa/histogram.cpp
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * Histogram tests
> + */
> +
> +#include "../src/ipa/libipa/histogram.h"
> +
> +#include <cmath>
> +#include <iostream>
> +#include <map>
> +#include <stdint.h>
> +
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +using namespace ipa;
> +
> +#define ASSERT_EQ(a, b)                    \
> +       if (!((a) == (b))) {               \
> +               printf(#a " != " #b "\n"); \
> +               return TestFail;           \
> +       }
> +
> +class HistogramTest : public Test
> +{
> +protected:
> +       int run()
> +       {
> +               auto hist = Histogram({ { 50, 50 } });
> +
> +               ASSERT_EQ(hist.bins(), 2);
> +               ASSERT_EQ(hist.total(), 100);
> +
> +               ASSERT_EQ(hist.cumulativeFrequency(1.0), 50);
> +               ASSERT_EQ(hist.cumulativeFrequency(1.5), 75);
> +               ASSERT_EQ(hist.cumulativeFrequency(2.0), 100);
> +
> +               ASSERT_EQ(hist.quantile(0.0), 0.0);
> +               ASSERT_EQ(hist.quantile(1.0), 2.0);
> +               ASSERT_EQ(hist.quantile(0.5), 1.0);
> +
> +               ASSERT_EQ(hist.interQuantileMean(0.0, 1.0), 1.0);
> +               ASSERT_EQ(hist.interQuantileMean(0.0, 0.5), 0.5);
> +               ASSERT_EQ(hist.interQuantileMean(0.5, 1.0), 1.5);
> +
> +               return TestPass;

I do like a good test to be sure objects are behaving...

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


> +       }
> +};
> +
> +TEST_REGISTER(HistogramTest)
> diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> index eaf4b49a187c..f094c1593f1b 100644
> --- a/test/ipa/libipa/meson.build
> +++ b/test/ipa/libipa/meson.build
> @@ -2,6 +2,7 @@
>  
>  libipa_test = [
>      {'name': 'fixedpoint', 'sources': ['fixedpoint.cpp']},
> +    {'name': 'histogram', 'sources': ['histogram.cpp']},
>      {'name': 'interpolator', 'sources': ['interpolator.cpp']},
>  ]
>  
> -- 
> 2.43.0
>
Laurent Pinchart March 31, 2025, 9:46 p.m. UTC | #2 
Hi Stefan,

Thank you for the patch.

On Mon, Mar 24, 2025 at 06:07:36PM +0100, Stefan Klug wrote:
> Add some basic tests for the histogram class.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  test/ipa/libipa/histogram.cpp | 53 +++++++++++++++++++++++++++++++++++
>  test/ipa/libipa/meson.build   |  1 +
>  2 files changed, 54 insertions(+)
>  create mode 100644 test/ipa/libipa/histogram.cpp
> 
> diff --git a/test/ipa/libipa/histogram.cpp b/test/ipa/libipa/histogram.cpp
> new file mode 100644
> index 000000000000..220a329e33b3
> --- /dev/null
> +++ b/test/ipa/libipa/histogram.cpp
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * Histogram tests
> + */
> +
> +#include "../src/ipa/libipa/histogram.h"
> +
> +#include <cmath>
> +#include <iostream>
> +#include <map>
> +#include <stdint.h>
> +
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +using namespace ipa;
> +
> +#define ASSERT_EQ(a, b)                    \
> +	if (!((a) == (b))) {               \
> +		printf(#a " != " #b "\n"); \

We use std::cout.

> +		return TestFail;           \
> +	}

One day we should really convert unit tests to use exceptions.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +class HistogramTest : public Test
> +{
> +protected:
> +	int run()
> +	{
> +		auto hist = Histogram({ { 50, 50 } });
> +
> +		ASSERT_EQ(hist.bins(), 2);
> +		ASSERT_EQ(hist.total(), 100);
> +
> +		ASSERT_EQ(hist.cumulativeFrequency(1.0), 50);
> +		ASSERT_EQ(hist.cumulativeFrequency(1.5), 75);
> +		ASSERT_EQ(hist.cumulativeFrequency(2.0), 100);
> +
> +		ASSERT_EQ(hist.quantile(0.0), 0.0);
> +		ASSERT_EQ(hist.quantile(1.0), 2.0);
> +		ASSERT_EQ(hist.quantile(0.5), 1.0);
> +
> +		ASSERT_EQ(hist.interQuantileMean(0.0, 1.0), 1.0);
> +		ASSERT_EQ(hist.interQuantileMean(0.0, 0.5), 0.5);
> +		ASSERT_EQ(hist.interQuantileMean(0.5, 1.0), 1.5);
> +
> +		return TestPass;
> +	}
> +};
> +
> +TEST_REGISTER(HistogramTest)
> diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> index eaf4b49a187c..f094c1593f1b 100644
> --- a/test/ipa/libipa/meson.build
> +++ b/test/ipa/libipa/meson.build
> @@ -2,6 +2,7 @@
>  
>  libipa_test = [
>      {'name': 'fixedpoint', 'sources': ['fixedpoint.cpp']},
> +    {'name': 'histogram', 'sources': ['histogram.cpp']},
>      {'name': 'interpolator', 'sources': ['interpolator.cpp']},
>  ]
>
Stefan Klug April 1, 2025, 9:40 a.m. UTC | #3 
Hi Laurent,

Thank you for the review. 

On Tue, Apr 01, 2025 at 12:46:58AM +0300, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Mon, Mar 24, 2025 at 06:07:36PM +0100, Stefan Klug wrote:
> > Add some basic tests for the histogram class.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  test/ipa/libipa/histogram.cpp | 53 +++++++++++++++++++++++++++++++++++
> >  test/ipa/libipa/meson.build   |  1 +
> >  2 files changed, 54 insertions(+)
> >  create mode 100644 test/ipa/libipa/histogram.cpp
> > 
> > diff --git a/test/ipa/libipa/histogram.cpp b/test/ipa/libipa/histogram.cpp
> > new file mode 100644
> > index 000000000000..220a329e33b3
> > --- /dev/null
> > +++ b/test/ipa/libipa/histogram.cpp
> > @@ -0,0 +1,53 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2024, Ideas on Board Oy
> > + *
> > + * Histogram tests
> > + */
> > +
> > +#include "../src/ipa/libipa/histogram.h"
> > +
> > +#include <cmath>
> > +#include <iostream>
> > +#include <map>
> > +#include <stdint.h>
> > +
> > +#include "test.h"
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +using namespace ipa;
> > +
> > +#define ASSERT_EQ(a, b)                    \
> > +	if (!((a) == (b))) {               \
> > +		printf(#a " != " #b "\n"); \
> 
> We use std::cout.

Ahrgh sure, I'll fix that.

> 
> > +		return TestFail;           \
> > +	}
> 
> One day we should really convert unit tests to use exceptions.

Or maybe just google test. That would prevent recreating our own test
framework. I was really missing their macros.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks,
Stefan
> 
> > +
> > +class HistogramTest : public Test
> > +{
> > +protected:
> > +	int run()
> > +	{
> > +		auto hist = Histogram({ { 50, 50 } });
> > +
> > +		ASSERT_EQ(hist.bins(), 2);
> > +		ASSERT_EQ(hist.total(), 100);
> > +
> > +		ASSERT_EQ(hist.cumulativeFrequency(1.0), 50);
> > +		ASSERT_EQ(hist.cumulativeFrequency(1.5), 75);
> > +		ASSERT_EQ(hist.cumulativeFrequency(2.0), 100);
> > +
> > +		ASSERT_EQ(hist.quantile(0.0), 0.0);
> > +		ASSERT_EQ(hist.quantile(1.0), 2.0);
> > +		ASSERT_EQ(hist.quantile(0.5), 1.0);
> > +
> > +		ASSERT_EQ(hist.interQuantileMean(0.0, 1.0), 1.0);
> > +		ASSERT_EQ(hist.interQuantileMean(0.0, 0.5), 0.5);
> > +		ASSERT_EQ(hist.interQuantileMean(0.5, 1.0), 1.5);
> > +
> > +		return TestPass;
> > +	}
> > +};
> > +
> > +TEST_REGISTER(HistogramTest)
> > diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> > index eaf4b49a187c..f094c1593f1b 100644
> > --- a/test/ipa/libipa/meson.build
> > +++ b/test/ipa/libipa/meson.build
> > @@ -2,6 +2,7 @@
> >  
> >  libipa_test = [
> >      {'name': 'fixedpoint', 'sources': ['fixedpoint.cpp']},
> > +    {'name': 'histogram', 'sources': ['histogram.cpp']},
> >      {'name': 'interpolator', 'sources': ['interpolator.cpp']},
> >  ]
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/test/ipa/libipa/histogram.cpp b/test/ipa/libipa/histogram.cpp
new file mode 100644
index 000000000000..220a329e33b3
--- /dev/null
+++ b/test/ipa/libipa/histogram.cpp
@@ -0,0 +1,53 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2024, Ideas on Board Oy
+ *
+ * Histogram tests
+ */
+
+#include "../src/ipa/libipa/histogram.h"
+
+#include <cmath>
+#include <iostream>
+#include <map>
+#include <stdint.h>
+
+#include "test.h"
+
+using namespace std;
+using namespace libcamera;
+using namespace ipa;
+
+#define ASSERT_EQ(a, b)                    \
+	if (!((a) == (b))) {               \
+		printf(#a " != " #b "\n"); \
+		return TestFail;           \
+	}
+
+class HistogramTest : public Test
+{
+protected:
+	int run()
+	{
+		auto hist = Histogram({ { 50, 50 } });
+
+		ASSERT_EQ(hist.bins(), 2);
+		ASSERT_EQ(hist.total(), 100);
+
+		ASSERT_EQ(hist.cumulativeFrequency(1.0), 50);
+		ASSERT_EQ(hist.cumulativeFrequency(1.5), 75);
+		ASSERT_EQ(hist.cumulativeFrequency(2.0), 100);
+
+		ASSERT_EQ(hist.quantile(0.0), 0.0);
+		ASSERT_EQ(hist.quantile(1.0), 2.0);
+		ASSERT_EQ(hist.quantile(0.5), 1.0);
+
+		ASSERT_EQ(hist.interQuantileMean(0.0, 1.0), 1.0);
+		ASSERT_EQ(hist.interQuantileMean(0.0, 0.5), 0.5);
+		ASSERT_EQ(hist.interQuantileMean(0.5, 1.0), 1.5);
+
+		return TestPass;
+	}
+};
+
+TEST_REGISTER(HistogramTest)
diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
index eaf4b49a187c..f094c1593f1b 100644
--- a/test/ipa/libipa/meson.build
+++ b/test/ipa/libipa/meson.build
@@ -2,6 +2,7 @@ 
 
 libipa_test = [
     {'name': 'fixedpoint', 'sources': ['fixedpoint.cpp']},
+    {'name': 'histogram', 'sources': ['histogram.cpp']},
     {'name': 'interpolator', 'sources': ['interpolator.cpp']},
 ]