diff --git a/CMakeLists.txt b/CMakeLists.txt index 995abd97..f8dab10a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -63,6 +63,7 @@ option(AARE_FETCH_CATCH "Use FetchContent to download catch2" ON) option(AARE_FETCH_JSON "Use FetchContent to download nlohmann::json" ON) option(AARE_FETCH_ZMQ "Use FetchContent to download libzmq" ON) option(AARE_FETCH_MINUIT2 "Use FetchContent to download Minuit2" ON) +option(AARE_OPENMP "Enable OpenMP support" ON) option(AARE_WARNINGS_AS_ERRORS "Treat warnings as errors during compilation" OFF) @@ -218,6 +219,19 @@ else() find_package(nlohmann_json 3.11.3 REQUIRED) endif() +if(AARE_OPENMP) + find_package( + OpenMP + COMPONENTS CXX + REQUIRED) + if(NOT OpenMP_CXX_FOUND) + message(STATUS "OpenMP not found. Please install OpenMP") # TODO: add fetch + # content + else() + message(STATUS "Found openmp ${OpenMP_CXX_VERSION}") + endif() +endif() + include(GNUInstallDirs) # If conda build, always set lib dir to 'lib' @@ -342,6 +356,7 @@ set(PUBLICHEADERS include/aare/Frame.hpp include/aare/MultiThreadedFileReader.hpp include/aare/hist/PixelHistogram.hpp + include/aare/hist/PixelHistogramOpenMP.hpp include/aare/hist/PixelHistogramImpl.hpp include/aare/hist/PedestalTrackingPixelHistogram.hpp include/aare/GainMap.hpp @@ -403,6 +418,12 @@ target_link_libraries( # helpers.cmake PRIVATE aare_compiler_flags Threads::Threads $) +if(AARE_OPENMP) + target_link_libraries( + aare_core PUBLIC OpenMP::OpenMP_CXX) # TODO: maybe add as PUBLIC if used in + # templated headers +endif() + target_include_directories( aare_core SYSTEM PRIVATE $) @@ -492,9 +513,8 @@ endif() add_custom_target( check-format COMMAND - find \\ (-name "*.cpp" -o -name "*.hpp" \\) -not -path "./build/*" | xargs - -I {} -n 1 -P 10 bash -c - "clang-format -Werror -style=\"file:.clang-format\" {} | diff {} -" + bash -c + [=[ find \( -name "*.cpp" -o -name "*.hpp" \) -not -path "./build/*" | xargs -I {} -n 1 -P 10 bash -c "clang-format -Werror -style=\"file:.clang-format\" {} | diff {} -" ]=] WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} COMMENT "Checking code formatting with clang-format" VERBATIM) @@ -502,8 +522,8 @@ add_custom_target( add_custom_target( format-files COMMAND - find \\ (-name "*.cpp" -o -name "*.hpp" \\) -not -path "./build/*" | xargs - -I {} -n 1 -P 10 bash -c "clang-format -i -style=\"file:.clang-format\" {}" + bash -c + [=[ find \( -name "*.cpp" -o -name "*.hpp" \) -not -path "./build/*" | xargs -I {} -n 1 -P 10 bash -c "clang-format -i -style=\"file:.clang-format\" {}" ]=] WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} COMMENT "Formatting with clang-format" VERBATIM) @@ -518,18 +538,9 @@ endif() add_custom_target( clang-tidy COMMAND - find \\ (-path - "./src/*" - -a - -not - -path - "./src/python/*" - -a - \\ - (-name "*.cpp" -not -name "*.test.cpp" \\) - \\) -not -name "CircularFifo.hpp" -not -name - "ProducerConsumerQueue.hpp" -not -name "VariableSizeClusterFinder.hpp" | - xargs -I {} -n 1 -P 10 bash -c + find -path "./src/*" -not -path "./src/python/*" -not -name "*.test.cpp" + -not -name "CircularFifo.hpp" -not -name "ProducerConsumerQueue.hpp" -not + -name "VariableSizeClusterFinder.hpp" | xargs -I {} -n 1 -P 10 bash -c "${CLANG_TIDY_COMMAND} --config-file=.clang-tidy -p build {}" WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} COMMENT "linting with clang-tidy" diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index ef378e4d..cc106802 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -19,7 +19,7 @@ add_executable(benchmarks) target_sources( benchmarks PRIVATE ndarray_benchmark.cpp calculateeta_benchmark.cpp - reduce_benchmark.cpp) + reduce_benchmark.cpp HistogramBenchmark.cpp) # Link Google Benchmark and other necessary libraries target_link_libraries(benchmarks PRIVATE benchmark::benchmark aare_core diff --git a/benchmarks/HistogramBenchmark.cpp b/benchmarks/HistogramBenchmark.cpp new file mode 100644 index 00000000..ec34d362 --- /dev/null +++ b/benchmarks/HistogramBenchmark.cpp @@ -0,0 +1,83 @@ +#include "aare/hist/PixelHistogram.hpp" +#include "aare/hist/PixelHistogramOpenMP.hpp" +#include +#include + +using namespace aare; + +class TestHistogram : public benchmark::Fixture { + public: + std::vector> images; + void SetUp(::benchmark::State &state) { + + std::cout << "In Fixture Setup" << std::endl; + + std::mt19937 gen(rd()); // to seed mersenne twister. + std::uniform_real_distribution dist(xmin, xmax); + + images.resize(num_images); + + // + + for (size_t i = 0; i < num_images; ++i) { + NDArray image({rows, cols}); + for (ssize_t r = 0; r < rows; ++r) { + for (ssize_t c = 0; c < cols; ++c) { + image(r, c) = dist(gen); + } + } + images[i] = std::move(image); + } + } + + // void TearDown(::benchmark::State& state) { + // } + + const ssize_t rows = 512; + const ssize_t cols = 1024; + const size_t n_bins = 200; + const float xmin = 600.0f; + const float xmax = 1400.0f; + const size_t num_images = 1000; + + private: + std::random_device rd{}; +}; + +BENCHMARK_DEFINE_F(TestHistogram, process_histogram_openmp) +(benchmark::State &st) { + const int num_threads = st.range(0); + + PixelHistogramOpenMP hist(rows, cols, n_bins, xmin, xmax, num_threads); + for (auto _ : st) { + // This code gets timed + for (const auto &img : images) { + hist.fill_async(NDArray(img)); + } + auto result = hist.values(); + benchmark::DoNotOptimize(result); + } +} + +BENCHMARK_DEFINE_F(TestHistogram, process_histogram)(benchmark::State &st) { + const int num_threads = st.range(0); + PixelHistogram hist(rows, cols, n_bins, xmin, xmax, num_threads); + for (auto _ : st) { + // This code gets timed + for (const auto &img : images) { + hist.fill_async(NDArray(img)); + } + auto result = hist.values(); + benchmark::DoNotOptimize(result); + } +} + +BENCHMARK_REGISTER_F(TestHistogram, process_histogram) + ->DenseRange(1, 20) + ->Iterations(10); // 1 to 20 threads + +BENCHMARK_REGISTER_F(TestHistogram, process_histogram_openmp) + ->DenseRange(1, 20) + ->Iterations(10); // 1 to 20 threads + +// BENCHMARK_MAIN(); \ No newline at end of file diff --git a/include/aare/hist/PixelHistogramOpenMP.hpp b/include/aare/hist/PixelHistogramOpenMP.hpp new file mode 100644 index 00000000..e22b349a --- /dev/null +++ b/include/aare/hist/PixelHistogramOpenMP.hpp @@ -0,0 +1,219 @@ +#pragma once +#include "aare/NDArray.hpp" +#include "aare/NDView.hpp" +#include "aare/ProducerConsumerQueue.hpp" +#include "aare/hist/PixelHistogramImpl.hpp" +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace aare { +template +class PixelHistogramOpenMP { + private: + using Hist = PixelHistogramImpl; + using AsyncQueue = ProducerConsumerQueue>; + + int rows_; + int cols_; + int n_threads_; + const AxisType xmin_; + const AxisType xmax_; + + // processor thread processing the images + std::thread process_thread; + + std::atomic m_done_processing_image = false; + + std::atomic m_stop_processing = false; + + // Async producer/consumer pipeline. SPSC queue feeds the coordinator + // thread, which fans each image out to the worker pool one at a time. + // TODO: batch processing? + // TODO: FIFO to avoid allocations? + std::unique_ptr async_queue_; + + std::chrono::microseconds async_wait_{100}; + + /** fill the histogram with images in async_queue*/ + void fill_histogram(); + + Hist histogram{}; + + public: + PixelHistogramOpenMP(int rows, int cols, int n_bins, AxisType xmin, + AxisType xmax, int n_threads = 1, + std::size_t max_pending = 16); + ~PixelHistogramOpenMP(); + + // Asynchronous fill: takes ownership of `image`, enqueues it for the + // coordinator thread, and returns. Blocks the caller only if the queue + // is full (single-producer, single-consumer queue with a sleep-poll + // backpressure loop, matching the convention in ClusterFinderMT). + void fill_async(NDArray &&image); + + // Wait for all queued async fills to complete. Cheap when the queue + // is already drained. + void flush() const; + + /** + * @brief Stop the processing thread and wait for it to finish. + */ + void stop(); + + // Implicitly flushes pending async fills first so the snapshot is + // consistent with everything that was submitted up to the call. + NDArray values() const; + NDArray bin_centers() const; + NDArray bin_edges() const; +}; + +template +PixelHistogramOpenMP::PixelHistogramOpenMP( + int rows, int cols, int n_bins, AxisType xmin, AxisType xmax, int n_threads, + std::size_t max_pending) + : rows_(rows), cols_(cols), n_threads_(n_threads), xmin_(xmin), xmax_(xmax), + histogram(rows, cols, n_bins, xmin, xmax) { + if (rows_ < 1 || cols_ < 1 || n_bins < 1) { + throw std::invalid_argument( + "PixelHistogram requires positive rows, cols and bins"); + } + if (n_threads < 1) { + throw std::invalid_argument( + "PixelHistogram requires at least one thread"); + } + if (max_pending < 1) { + throw std::invalid_argument("PixelHistogram requires max_pending >= 1"); + } + + n_threads_ = std::min(n_threads_, rows_ * cols_); + + // Async pipeline. The PCQ holds (size - 1) usable slots, so size up by + // one to honour the requested max_pending. + async_queue_ = std::make_unique( + static_cast(max_pending + 1)); + + process_thread = std::thread( + &PixelHistogramOpenMP::fill_histogram, this); +} + +template +PixelHistogramOpenMP::~PixelHistogramOpenMP() { + // Drain any pending async fills before tearing down the worker pool. + // The coordinator's loop keeps processing while stop_coordinator_ is + // true as long as the queue is non-empty (mirrors ClusterFinderMT). + + if (process_thread.joinable()) { + stop(); + process_thread.join(); + } +} + +template +void PixelHistogramOpenMP::fill_histogram() { + + omp_set_num_threads(n_threads_); + + NDArray image; + +#pragma omp parallel + while (!m_stop_processing) { + +#pragma omp single + { + while (!async_queue_->read(image) && !m_stop_processing) { + std::this_thread::sleep_for(async_wait_); + } + m_done_processing_image = m_stop_processing ? true : false; + } + +#pragma omp barrier + + if (m_stop_processing) + break; + +// set OMP_WAIT_POLICY=PASSIVE +#pragma omp for + for (int row = 0; row < rows_; ++row) { + for (ssize_t col = 0; col < cols_; ++col) { + const auto val = image(row, col); + histogram.fill_unchecked(row, static_cast(col), val); + } + } + +#pragma omp single + { + m_done_processing_image = true; + } + } +} + +template +NDArray +PixelHistogramOpenMP::values() const { + // Make sure any pending async fills are merged in before we snapshot + // the partial histograms. Cheap when the queue is already drained. + flush(); + + // TODO: maybe its even better to return a view + NDArray data(histogram.view()); + + return data; +} + +template +void PixelHistogramOpenMP::fill_async( + NDArray &&image) { + + if (image.shape(0) != rows_ || image.shape(1) != cols_) { + throw std::invalid_argument("PixelHistogram image shape does " + "not match constructor shape"); + } + + // SPSC backpressure: spin with a short sleep until a slot frees up. + // The std::move only consumes `image` on the iteration that + // succeeds (placement-new inside write() runs only when the slot is + // free). + while (!async_queue_->write(std::move(image))) { + std::this_thread::sleep_for(async_wait_); + } +} + +template +void PixelHistogramOpenMP::flush() const { + while (!async_queue_->isEmpty() || !m_done_processing_image) { + std::this_thread::sleep_for(async_wait_); + } +} + +template +void PixelHistogramOpenMP::stop() { + flush(); + m_stop_processing = true; +} + +template +NDArray +PixelHistogramOpenMP::bin_centers() const { + return histogram.bin_centers(); +} + +template +NDArray +PixelHistogramOpenMP::bin_edges() const { + return histogram.bin_edges(); +} + +} // namespace aare diff --git a/src/hist/PixelHistogram.test.cpp b/src/hist/PixelHistogram.test.cpp index d681c789..32ea124b 100644 --- a/src/hist/PixelHistogram.test.cpp +++ b/src/hist/PixelHistogram.test.cpp @@ -13,11 +13,14 @@ #include "test_macros.hpp" #include "aare/hist/PixelHistogram.hpp" +#include "aare/hist/PixelHistogramOpenMP.hpp" using aare::NDArray; using aare::NDView; using aare::PixelHistogram; +using aare::PixelHistogramOpenMP; + namespace { // The synchronous fill() has been removed; fill_async() is the only entry // point. This helper submits one frame and blocks until it has been merged @@ -25,8 +28,8 @@ namespace { } // namespace -TEST_CASE("Fill one pixel of a 5x10 histogram") { - PixelHistogram hist(5, 10, 20, 0.0f, 10.0f); +TEST_CASE("Fill one pixel of a 5x10 histogram", "[histogram]") { + NDArray image( {5, 10}, -1.0f); // Need to fill with -1 to not generate counts @@ -35,32 +38,63 @@ TEST_CASE("Fill one pixel of a 5x10 histogram") { // fill_blocking(hist, image.view()); - hist.fill_async(NDArray(image)); - hist.flush(); // Wait for the async fill to complete before we check the - // results + SECTION("std::thread") { + PixelHistogram hist(5, 10, 20, 0.0f, 10.0f); - auto values = hist.values(); - REQUIRE(values.shape(0) == 5); - REQUIRE(values.shape(1) == 10); - REQUIRE(values.shape(2) == 20); + hist.fill_async(NDArray(image)); + hist.flush(); // Wait for the async fill to complete before we check the + // results - // Check that the correct bin for pixel (2,3) has count 1 - CHECK(values(2, 3, 11) == 1); + auto values = hist.values(); + REQUIRE(values.shape(0) == 5); + REQUIRE(values.shape(1) == 10); + REQUIRE(values.shape(2) == 20); - // Check that all other bins are zero - for (ssize_t row = 0; row < values.shape(0); ++row) { - for (ssize_t col = 0; col < values.shape(1); ++col) { - for (ssize_t bin = 0; bin < values.shape(2); ++bin) { - if (!(row == 2 && col == 3 && bin == 11)) { - CHECK(values(row, col, bin) == 0); + // Check that the correct bin for pixel (2,3) has count 1 + CHECK(values(2, 3, 11) == 1); + + // Check that all other bins are zero + for (ssize_t row = 0; row < values.shape(0); ++row) { + for (ssize_t col = 0; col < values.shape(1); ++col) { + for (ssize_t bin = 0; bin < values.shape(2); ++bin) { + if (!(row == 2 && col == 3 && bin == 11)) { + CHECK(values(row, col, bin) == 0); + } + } + } + } + } + SECTION("OpenMP") { + PixelHistogramOpenMP hist(5, 10, 20, 0.0f, 10.0f, 2); + + hist.fill_async(NDArray(image)); + hist.flush(); // Wait for the async fill to complete before we check the + // results + + auto values = hist.values(); + REQUIRE(values.shape(0) == 5); + REQUIRE(values.shape(1) == 10); + REQUIRE(values.shape(2) == 20); + + // Check that the correct bin for pixel (2,3) has count 1 + CHECK(values(2, 3, 11) == 1); + + // Check that all other bins are zero + for (ssize_t row = 0; row < values.shape(0); ++row) { + for (ssize_t col = 0; col < values.shape(1); ++col) { + for (ssize_t bin = 0; bin < values.shape(2); ++bin) { + if (!(row == 2 && col == 3 && bin == 11)) { + CHECK(values(row, col, bin) == 0); + } } } } } } -TEST_CASE("Fill pixels with uneven partial histogram row slices") { - PixelHistogram hist(5, 4, 10, 0.0f, 10.0f, 3); +TEST_CASE("Fill pixels with uneven partial histogram row slices", + "[histogram]") { + NDArray image({5, 4}, -1.0f); image(0, 0) = 0.2f; @@ -69,37 +103,75 @@ TEST_CASE("Fill pixels with uneven partial histogram row slices") { image(3, 3) = 3.2f; image(4, 0) = 4.2f; - hist.fill_async(NDArray(image)); - hist.flush(); + SECTION("std::thread") { - auto values = hist.values(); - REQUIRE(values.shape(0) == 5); - REQUIRE(values.shape(1) == 4); - REQUIRE(values.shape(2) == 10); + PixelHistogram hist(5, 4, 10, 0.0f, 10.0f, 3); - CHECK(values(0, 0, 0) == 1); - CHECK(values(1, 1, 1) == 1); - CHECK(values(2, 2, 2) == 1); - CHECK(values(3, 3, 3) == 1); - CHECK(values(4, 0, 4) == 1); + hist.fill_async(NDArray(image)); + hist.flush(); - for (ssize_t row = 0; row < values.shape(0); ++row) { - for (ssize_t col = 0; col < values.shape(1); ++col) { - for (ssize_t bin = 0; bin < values.shape(2); ++bin) { - const bool expected = (row == 0 && col == 0 && bin == 0) || - (row == 1 && col == 1 && bin == 1) || - (row == 2 && col == 2 && bin == 2) || - (row == 3 && col == 3 && bin == 3) || - (row == 4 && col == 0 && bin == 4); - if (!expected) { - CHECK(values(row, col, bin) == 0); + auto values = hist.values(); + REQUIRE(values.shape(0) == 5); + REQUIRE(values.shape(1) == 4); + REQUIRE(values.shape(2) == 10); + + CHECK(values(0, 0, 0) == 1); + CHECK(values(1, 1, 1) == 1); + CHECK(values(2, 2, 2) == 1); + CHECK(values(3, 3, 3) == 1); + CHECK(values(4, 0, 4) == 1); + + for (ssize_t row = 0; row < values.shape(0); ++row) { + for (ssize_t col = 0; col < values.shape(1); ++col) { + for (ssize_t bin = 0; bin < values.shape(2); ++bin) { + const bool expected = (row == 0 && col == 0 && bin == 0) || + (row == 1 && col == 1 && bin == 1) || + (row == 2 && col == 2 && bin == 2) || + (row == 3 && col == 3 && bin == 3) || + (row == 4 && col == 0 && bin == 4); + if (!expected) { + CHECK(values(row, col, bin) == 0); + } + } + } + } + } + SECTION("OpenMP") { + PixelHistogramOpenMP hist(5, 4, 10, 0.0f, 10.0f, 3); + + hist.fill_async(NDArray(image)); + hist.flush(); + + auto values = hist.values(); + REQUIRE(values.shape(0) == 5); + REQUIRE(values.shape(1) == 4); + REQUIRE(values.shape(2) == 10); + + CHECK(values(0, 0, 0) == 1); + CHECK(values(1, 1, 1) == 1); + CHECK(values(2, 2, 2) == 1); + CHECK(values(3, 3, 3) == 1); + CHECK(values(4, 0, 4) == 1); + + for (ssize_t row = 0; row < values.shape(0); ++row) { + for (ssize_t col = 0; col < values.shape(1); ++col) { + for (ssize_t bin = 0; bin < values.shape(2); ++bin) { + const bool expected = (row == 0 && col == 0 && bin == 0) || + (row == 1 && col == 1 && bin == 1) || + (row == 2 && col == 2 && bin == 2) || + (row == 3 && col == 3 && bin == 3) || + (row == 4 && col == 0 && bin == 4); + if (!expected) { + CHECK(values(row, col, bin) == 0); + } } } } } } -TEST_CASE("Row partitioning handles rows < n_threads * ceil(rows/n_threads)") { +TEST_CASE("Row partitioning handles rows < n_threads * ceil(rows/n_threads)", + "[histogram]") { // Regression test for the pre-existing row partitioning bug: with the // old ceil(rows / n_threads) scheme, rows=17 and n_threads=8 left // trailing threads with negative row counts and threw @@ -154,7 +226,7 @@ TEST_CASE("Row partitioning handles rows < n_threads * ceil(rows/n_threads)") { } } -TEST_CASE("Random fills match a reference implementation") { +TEST_CASE("Random fills match a reference implementation", "[histogram]") { // End-to-end correctness check: compare the implementation against a // simple per-pixel reference for several thread counts. Values are // sampled slightly wider than [xmin, xmax) so the out-of-range filter @@ -166,8 +238,6 @@ TEST_CASE("Random fills match a reference implementation") { constexpr float xmax = 4.5f; const int n_threads = GENERATE(1, 2, 4, 8); - PixelHistogram hist(rows, cols, n_bins, xmin, xmax, n_threads); - std::mt19937 rng(0xC0FFEE); std::uniform_real_distribution dist(xmin - 0.5f, xmax + 0.5f); @@ -202,40 +272,75 @@ TEST_CASE("Random fills match a reference implementation") { } } - for (const auto &img : frames) { - hist.fill_async(NDArray(img)); - } - hist.flush(); - auto h = hist.values(); + SECTION("std::thread") { + PixelHistogram hist(rows, cols, n_bins, xmin, xmax, n_threads); - REQUIRE(h.shape(0) == rows); - REQUIRE(h.shape(1) == cols); - REQUIRE(h.shape(2) == n_bins); + for (const auto &img : frames) { + hist.fill_async(NDArray(img)); + } + hist.flush(); + auto h = hist.values(); - bool all_match = true; - for (ssize_t r = 0; r < rows && all_match; ++r) { - for (ssize_t c = 0; c < cols && all_match; ++c) { - for (ssize_t b = 0; b < n_bins && all_match; ++b) { - if (h(r, c, b) != expected(r, c, b)) { - all_match = false; - INFO("n_threads=" << n_threads << " r=" << r << " c=" << c - << " b=" << b << " got=" << h(r, c, b) - << " expected=" << expected(r, c, b)); - CHECK(h(r, c, b) == expected(r, c, b)); + REQUIRE(h.shape(0) == rows); + REQUIRE(h.shape(1) == cols); + REQUIRE(h.shape(2) == n_bins); + + bool all_match = true; + for (ssize_t r = 0; r < rows && all_match; ++r) { + for (ssize_t c = 0; c < cols && all_match; ++c) { + for (ssize_t b = 0; b < n_bins && all_match; ++b) { + if (h(r, c, b) != expected(r, c, b)) { + all_match = false; + INFO("n_threads=" << n_threads << " r=" << r + << " c=" << c << " b=" << b + << " got=" << h(r, c, b) + << " expected=" << expected(r, c, b)); + CHECK(h(r, c, b) == expected(r, c, b)); + } } } } + CHECK(all_match); + } + SECTION("OpenMP") { + PixelHistogramOpenMP hist(rows, cols, n_bins, xmin, xmax, n_threads); + + for (const auto &img : frames) { + hist.fill_async(NDArray(img)); + } + hist.flush(); + auto h = hist.values(); + + REQUIRE(h.shape(0) == rows); + REQUIRE(h.shape(1) == cols); + REQUIRE(h.shape(2) == n_bins); + + bool all_match = true; + for (ssize_t r = 0; r < rows && all_match; ++r) { + for (ssize_t c = 0; c < cols && all_match; ++c) { + for (ssize_t b = 0; b < n_bins && all_match; ++b) { + if (h(r, c, b) != expected(r, c, b)) { + all_match = false; + INFO("n_threads=" << n_threads << " r=" << r + << " c=" << c << " b=" << b + << " got=" << h(r, c, b) + << " expected=" << expected(r, c, b)); + CHECK(h(r, c, b) == expected(r, c, b)); + } + } + } + } + CHECK(all_match); } - CHECK(all_match); } -TEST_CASE("fill_async with mismatched shape throws") { +TEST_CASE("fill_async with mismatched shape throws", "[histogram]") { PixelHistogram hist(8, 8, 16, 0.0f, 1.0f, 2); NDArray bad({4, 4}, 0.0f); CHECK_THROWS_AS(hist.fill_async(std::move(bad)), std::invalid_argument); } -TEST_CASE("Destructor drains pending async fills") { +TEST_CASE("Destructor drains pending async fills", "[histogram]") { // Submit more frames than the queue can hold so backpressure kicks in, // then immediately let the histogram go out of scope and verify that // the merged values() matches the reference computed sequentially. @@ -259,44 +364,85 @@ TEST_CASE("Destructor drains pending async fills") { frames.push_back(std::move(img)); } - NDArray snapshot({rows, cols, n_bins}, uint16_t{0}); - { - PixelHistogram hist(rows, cols, n_bins, xmin, xmax, 2, max_pending); - for (auto &img : frames) { - // Move a copy so we can also build the reference below. - NDArray copy({rows, cols}, 0.0f); - std::memcpy(copy.data(), img.data(), copy.total_bytes()); - hist.fill_async(std::move(copy)); + SECTION("std::thread") { + NDArray snapshot({rows, cols, n_bins}, uint16_t{0}); + { + PixelHistogram hist(rows, cols, n_bins, xmin, xmax, 2, max_pending); + for (auto &img : frames) { + // Move a copy so we can also build the reference below. + NDArray copy({rows, cols}, 0.0f); + std::memcpy(copy.data(), img.data(), copy.total_bytes()); + hist.fill_async(std::move(copy)); + } + // No explicit flush(); destructor must drain. + // Capture values() *after* the loop but inside the scope so it + // observes everything that was submitted (values flushes too). + snapshot = hist.values(); } - // No explicit flush(); destructor must drain. - // Capture values() *after* the loop but inside the scope so it - // observes everything that was submitted (values flushes too). - snapshot = hist.values(); - } - PixelHistogram reference(rows, cols, n_bins, xmin, xmax, 2); - for (const auto &img : frames) - reference.fill_async(NDArray(img.view())); - auto expected = reference.values(); + PixelHistogram reference(rows, cols, n_bins, xmin, xmax, 2); + for (const auto &img : frames) + reference.fill_async(NDArray(img.view())); + auto expected = reference.values(); - bool all_match = true; - for (ssize_t r = 0; r < rows && all_match; ++r) { - for (ssize_t c = 0; c < cols && all_match; ++c) { - for (ssize_t b = 0; b < n_bins && all_match; ++b) { - if (snapshot(r, c, b) != expected(r, c, b)) { - all_match = false; - INFO("r=" << r << " c=" << c << " b=" << b - << " got=" << snapshot(r, c, b) - << " expected=" << expected(r, c, b)); - CHECK(snapshot(r, c, b) == expected(r, c, b)); + bool all_match = true; + for (ssize_t r = 0; r < rows && all_match; ++r) { + for (ssize_t c = 0; c < cols && all_match; ++c) { + for (ssize_t b = 0; b < n_bins && all_match; ++b) { + if (snapshot(r, c, b) != expected(r, c, b)) { + all_match = false; + INFO("r=" << r << " c=" << c << " b=" << b + << " got=" << snapshot(r, c, b) + << " expected=" << expected(r, c, b)); + CHECK(snapshot(r, c, b) == expected(r, c, b)); + } } } } + CHECK(all_match); + } + SECTION("OpenMP") { + NDArray snapshot({rows, cols, n_bins}, uint16_t{0}); + { + PixelHistogramOpenMP hist(rows, cols, n_bins, xmin, xmax, 2, + max_pending); + for (auto &img : frames) { + // Move a copy so we can also build the reference below. + NDArray copy({rows, cols}, 0.0f); + std::memcpy(copy.data(), img.data(), copy.total_bytes()); + hist.fill_async(std::move(copy)); + } + // No explicit flush(); destructor must drain. + // Capture values() *after* the loop but inside the scope so it + // observes everything that was submitted (values flushes too). + snapshot = hist.values(); + } + + PixelHistogramOpenMP reference(rows, cols, n_bins, xmin, xmax, 2); + for (const auto &img : frames) + reference.fill_async(NDArray(img.view())); + auto expected = reference.values(); + + bool all_match = true; + for (ssize_t r = 0; r < rows && all_match; ++r) { + for (ssize_t c = 0; c < cols && all_match; ++c) { + for (ssize_t b = 0; b < n_bins && all_match; ++b) { + if (snapshot(r, c, b) != expected(r, c, b)) { + all_match = false; + INFO("r=" << r << " c=" << c << " b=" << b + << " got=" << snapshot(r, c, b) + << " expected=" << expected(r, c, b)); + CHECK(snapshot(r, c, b) == expected(r, c, b)); + } + } + } + } + CHECK(all_match); } - CHECK(all_match); } -TEST_CASE("PixelHistogram supports custom storage and axis types") { +TEST_CASE("PixelHistogram supports custom storage and axis types", + "[histogram]") { PixelHistogram hist(2, 2, 4, 0.0, 4.0, 1); NDArray image({2, 2}, -1.0);