From dd409cfe41eee9623e8485ec24410580fae2d616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= Date: Mon, 7 Sep 2026 08:19:26 +0200 Subject: [PATCH] Fixed ClusterVector move, filtering, and improved documentation (#357) - Default `ClusterVector` move operations simplifying the code and fixing a bug. - Fixed inconsistent frame number type (uint64/int32) - Validate Python masks as one-dimensional, C-contiguous Boolean arrays, handle empty masks safely, and reserve filtered storage based on the selected cluster count. - Align the C++ and Python API documentation with the implementation, including concise `hitmap` and reduction documentation and a correctly rendered constructor example. --- RELEASE.md | 6 + docs/src/python/cluster/pyClusterVector.rst | 64 +++++-- include/aare/Cluster.hpp | 25 +-- include/aare/ClusterVector.hpp | 114 +++++++---- python/aare/ClusterVector.py | 31 ++- python/src/bind_Cluster.hpp | 37 ++-- python/src/bind_ClusterVector.hpp | 200 +++++++++++++------- python/src/module.cpp | 6 - python/tests/test_ClusterVector.py | 33 +++- src/Cluster.test.cpp | 21 +- src/ClusterVector.test.cpp | 100 ++++++++-- 11 files changed, 450 insertions(+), 187 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index ab7cee36..bb3cb855 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -43,6 +43,12 @@ - ``TimingMode::Auto`` changed to ``TimingMode::AUTO_TIMING``, ``TimingMode::Trigger`` changed to ``TimingMode::TRIGGER_EXPOSURE`` ### Bugfixes: +- Fixed ``ClusterVector`` move operations to transfer storage instead of + copying every cluster. +- Validate that ``ClusterVector`` masks are one-dimensional, C-contiguous + Boolean arrays. +- Preserve signed ``ClusterVector`` frame numbers when filtering or reducing + cluster dimensions. - Fixed broken reading of old (pre reordering) Moench03 - Supports reading all timing modes supported in slsDetectorPackage (auto, trigger, gating, burst_trigger, trigger_gating) diff --git a/docs/src/python/cluster/pyClusterVector.rst b/docs/src/python/cluster/pyClusterVector.rst index 1aaf1b78..fc9508c3 100644 --- a/docs/src/python/cluster/pyClusterVector.rst +++ b/docs/src/python/cluster/pyClusterVector.rst @@ -3,17 +3,16 @@ ClusterVector ================ -The ClusterVector, holds clusters from the ClusterFinder. Since it is templated -in C++ we use a suffix indicating the type of cluster it holds. The suffix follows -the same pattern as for ClusterFile i.e. ``ClusterVector_Cluster3x3i`` -for a vector holding 3x3 integer clusters. +A ClusterVector stores fixed-size clusters contiguously. Since it is templated +in C++, each bound class has a suffix indicating the cluster type. The suffix +follows the same pattern as ClusterFile; for example, +``ClusterVector_Cluster3x3i`` stores 3x3 clusters with 32-bit integer pixels. -At the moment the functionality from python is limited and it is not supported -to push_back clusters to the vector. The intended use case is to pass it to -C++ functions that support the ClusterVector or to view it as a numpy array. +The intended use case is to pass a ClusterVector to C++ functions that support +it or to view it as a NumPy array. -**View ClusterVector as numpy array** +**View ClusterVector as a NumPy array** .. code:: python @@ -25,7 +24,16 @@ C++ functions that support the ClusterVector or to view it as a numpy array. clusters = np.array(cluster_vector) # Avoid copying the data by passing copy=False - clusters = np.array(cluster_vector, copy = False) + clusters = np.array(cluster_vector, copy=False) + +.. warning:: + + A NumPy array created with ``copy=False`` is a view of the ClusterVector's + current storage. Do not call ``push_back`` or otherwise change the + ClusterVector while using the view. A ``push_back`` that reallocates the + backing buffer leaves existing NumPy views pointing to invalid memory, and + appending without reallocation does not update their shape. Use + ``copy=True`` if the ClusterVector may change after creating the array. .. py:currentmodule:: aare @@ -35,7 +43,8 @@ C++ functions that support the ClusterVector or to view it as a numpy array. :undoc-members: :inherited-members: -Below is the API of the ClusterVector_Cluster3x3i but all variants share the same API. +Below is the API of ``ClusterVector_Cluster3x3i``. All variants share the same +API. .. autoclass:: aare._aare.ClusterVector_Cluster3x3i :special-members: __init__, __call__ @@ -45,14 +54,39 @@ Below is the API of the ClusterVector_Cluster3x3i but all variants share the sam :inherited-members: -**Free Functions:** +**Free Functions:** -.. autofunction:: reduce_to_3x3 +.. py:function:: hitmap(image_size, clusters) :noindex: - Reduce a single Cluster to 3x3 by taking the 3x3 subcluster with highest photon energy. + Count cluster centers into an ``int32`` image. ``image_size`` is given as + ``(rows, columns)``, and output element ``[y, x]`` contains the number of + photon hits at that coordinate. Out-of-bounds hits are ignored. All registered + ClusterVector variants are accepted. -.. autofunction:: reduce_to_2x2 + :param tuple[int, int] image_size: Shape of the output image. + :param ClusterVector clusters: Clusters whose centers are counted. + :return: Hit counts with shape ``image_size``. + :rtype: numpy.ndarray + +.. py:function:: reduce_to_3x3(clustervector) :noindex: - Reduce a single Cluster to 2x2 by taking the 2x2 subcluster with highest photon energy. + Return a new vector containing the central 3x3 block of every input cluster. + Cluster order, coordinates, frame number, and pixel dtype are preserved. + + :param ClusterVector clustervector: Input clusters with both dimensions at + least 3 and at least one dimension greater than 3. + :return: Reduced 3x3 clusters. + :rtype: ClusterVector + +.. py:function:: reduce_to_2x2(clustervector) + :noindex: + + Return a new vector containing the highest-sum center-adjacent 2x2 block of + every input cluster. Cluster order, coordinates, frame number, and pixel + dtype are preserved. + + :param ClusterVector clustervector: Input clusters of size 2x2 or larger. + :return: Reduced 2x2 clusters. + :rtype: ClusterVector diff --git a/include/aare/Cluster.hpp b/include/aare/Cluster.hpp index aa56defe..b08a2878 100755 --- a/include/aare/Cluster.hpp +++ b/include/aare/Cluster.hpp @@ -52,9 +52,9 @@ struct Cluster { // TODO: handle 1 dimensional clusters /** - * @brief sum of 2x2 subcluster with highest energy - * @return photon energy of subcluster, 2x2 subcluster index relative to - * cluster center + * @brief Find the highest-sum center-adjacent 2x2 subcluster. + * @return Sum and corner index of the selected 2x2 subcluster relative to + * the cluster center */ Sum_index_pair max_sum_2x2() const { @@ -117,13 +117,10 @@ struct Cluster { }; /** - * @brief Reduce a cluster to a 2x2 cluster by selecting the 2x2 block with the - * highest sum. + * @brief Reduce a cluster to its highest-sum center-adjacent 2x2 block. * @param c Cluster to reduce - * @return reduced cluster + * @return Reduced cluster with the input coordinates * @note The cluster is filled using row major ordering starting at the top-left - * (thus for a max subcluster in the top left cornern the photon hit is at - * the fourth position) */ template @@ -194,17 +191,21 @@ Cluster reduce_to_2x2(const Cluster &c) { } /** - * @brief Reduce a cluster to a 3x3 cluster + * @brief Reduce a cluster to the 3x3 block around its center index. * @param c Cluster to reduce - * @return reduced cluster + * @pre ClusterSizeX and ClusterSizeY must both be at least 3, and at least one + * must be greater than 3. + * @return Reduced cluster with the input coordinates are preserved. */ template Cluster reduce_to_3x3(const Cluster &c) { - static_assert(ClusterSizeX >= 3 && ClusterSizeY >= 3, - "Cluster sizes must be at least 3x3 for reduction to 3x3"); + static_assert(ClusterSizeX >= 3 && ClusterSizeY >= 3 && + (ClusterSizeX > 3 || ClusterSizeY > 3), + "Cluster sizes must both be at least 3, and at least one " + "must be greater than 3 for reduction to 3x3"); Cluster result{}; diff --git a/include/aare/ClusterVector.hpp b/include/aare/ClusterVector.hpp index a4141857..37956933 100644 --- a/include/aare/ClusterVector.hpp +++ b/include/aare/ClusterVector.hpp @@ -20,14 +20,19 @@ template > { /** * @brief Construct a new ClusterVector object - * @param capacity initial capacity of the buffer in number of clusters + * @param capacity minimum initial capacity in number of clusters * @param frame_number frame number of the clusters. Default is 0, which is * also used to indicate that the clusters come from many frames */ - ClusterVector(size_t capacity = 1024, uint64_t frame_number = 0) + ClusterVector(size_t capacity = 1024, int32_t frame_number = 0) : m_frame_number(frame_number) { m_data.reserve(capacity); } @@ -57,18 +62,24 @@ class ClusterVector> { ClusterVector &operator=(ClusterVector &&other) noexcept = default; /** - * @brief Create a copy of the clustervector by filtering clusters in the - * ClusterVector using a boolean mask. - * @param mask boolean 1d mask - * @return ClusterVector containing only the clusters where the mask is - * true + * @brief Return a filtered copy selected by a one-dimensional Boolean mask. + * @param mask boolean 1d mask true if element in ClusterVector will be + * included + * @return ClusterVector containing the selected clusters in their original + * order and with the original frame number + * @throws std::runtime_error if the mask length differs from size() */ ClusterVector operator()(NDView mask) { if (static_cast(mask.size()) != m_data.size()) { throw std::runtime_error( LOCATION + "Mask size does not match number of clusters"); } - ClusterVector result(capacity(), frame_number()); + if (m_data.empty()) { + return ClusterVector(0, frame_number()); + } + const auto selected = + static_cast(std::count(mask.begin(), mask.end(), true)); + ClusterVector result(selected, frame_number()); for (size_t i = 0; i < m_data.size(); ++i) { if (mask(i)) { result.push_back(m_data[i]); @@ -77,20 +88,9 @@ class ClusterVector> { return result; } - // // Move assignment operator - // ClusterVector &operator=(ClusterVector &&other) noexcept { - // if (this != &other) { - // m_data = other.m_data; - // m_frame_number = other.m_frame_number; - // other.m_data.clear(); - // other.m_frame_number = 0; - // } - // return *this; - // } - /** - * @brief Sum the pixels in each cluster - * @return std::vector vector of sums for each cluster + * @brief Sum the pixels in each cluster. + * @return One sum for every cluster, in container order */ std::vector sum() { std::vector sums(m_data.size()); @@ -103,9 +103,10 @@ class ClusterVector> { } /** - * @brief Sum the pixels in the 2x2 subcluster with the biggest pixel sum in - * each cluster - * @return vector of sums index pairs for each cluster + * @brief Find the highest-sum center-adjacent 2x2 subcluster in each + * cluster. + * @return One sum and corner-index pair for every cluster, in container + * order */ std::vector> sum_2x2() { std::vector> sums_2x2(m_data.size()); @@ -125,10 +126,29 @@ class ClusterVector> { */ void reserve(size_t capacity) { m_data.reserve(capacity); } + /** + * @brief Change the number of stored clusters. + * @param size new number of clusters + * @note Growing the vector value-initializes new clusters and can + * invalidate pointers, references, and iterators. + */ void resize(size_t size) { m_data.resize(size); } + /** + * @brief Append a cluster to the vector. + * @param cluster cluster to append + * @note Reallocation invalidates pointers, references, iterators, and + * zero-copy NumPy views of the storage. + */ void push_back(const ClusterType &cluster) { m_data.push_back(cluster); } + /** + * @brief Append all clusters from another vector. + * @param other vector whose clusters are appended + * @return Reference to this vector + * @note The frame number of this vector is unchanged. + * @warning other must not refer to this vector. + */ ClusterVector &operator+=(const ClusterVector &other) { m_data.insert(m_data.end(), other.begin(), other.end()); @@ -145,8 +165,10 @@ class ClusterVector> { */ bool empty() const { return m_data.empty(); } + /** @brief Return the cluster size in the x dimension. */ uint8_t cluster_size_x() const { return ClusterSizeX; } + /** @brief Return the cluster size in the y dimension. */ uint8_t cluster_size_y() const { return ClusterSizeY; } /** @@ -161,7 +183,7 @@ class ClusterVector> { auto end() const { return m_data.end(); } /** - * @brief Return the size in bytes of a single cluster + * @brief Return the size in bytes of one stored cluster, including padding. */ size_t item_size() const { return sizeof(ClusterType); // 2 * sizeof(CoordType) + ClusterSizeX * @@ -172,8 +194,8 @@ class ClusterVector> { ClusterType const *data() const { return m_data.data(); } /** - * @brief Return a reference to the i-th cluster casted to type V - * @tparam V type of the cluster + * @brief Return a reference to the i-th cluster without bounds checking. + * @param i zero-based cluster index */ ClusterType &operator[](size_t i) { return m_data[i]; } @@ -185,16 +207,20 @@ class ClusterVector> { */ int32_t frame_number() const { return m_frame_number; } + /** + * @brief Set the signed 32-bit frame number associated with the clusters. + * @param frame_number frame number, or 0 for clusters from multiple frames + */ void set_frame_number(int32_t frame_number) { m_frame_number = frame_number; } }; /** - * @brief Reduce a cluster to a 2x2 cluster by selecting the 2x2 block with the - * highest sum. - * @param cv Clustervector containing clusters to reduce - * @return Clustervector with reduced clusters + * @brief Reduce every cluster to its highest-sum center-adjacent 2x2 block. + * @param cv ClusterVector containing clusters to reduce + * @return ClusterVector of 2x2 clusters in the original order and with the + * original frame number * @note The cluster is filled using row major ordering starting at the top-left * (thus for a max subcluster in the top left cornern the photon hit is at * the fourth position) @@ -204,7 +230,8 @@ template > reduce_to_2x2( const ClusterVector> &cv) { - ClusterVector> result; + ClusterVector> result(cv.size(), + cv.frame_number()); for (const auto &c : cv) { result.push_back(reduce_to_2x2(c)); } @@ -212,20 +239,25 @@ ClusterVector> reduce_to_2x2( } /** - * @brief Reduce a cluster to a 3x3 cluster - * @param cv Clustervector containing clusters to reduce - * @return Clustervector with reduced clusters + * @brief Reduce every cluster to the 3x3 block around its center index. + * @param cv ClusterVector containing clusters to reduce + * @pre ClusterSizeX and ClusterSizeY must both be at least 3, and at least one + * must be greater than 3. + * @return ClusterVector of 3x3 clusters in the original order and with the + * original frame number + * @note Coordinates are preserved. */ template ClusterVector> reduce_to_3x3( const ClusterVector> &cv) { - ClusterVector> result; + ClusterVector> result(cv.size(), + cv.frame_number()); for (const auto &c : cv) { result.push_back(reduce_to_3x3(c)); } return result; } -} // namespace aare \ No newline at end of file +} // namespace aare diff --git a/python/aare/ClusterVector.py b/python/aare/ClusterVector.py index a294bb55..6c4a65f9 100644 --- a/python/aare/ClusterVector.py +++ b/python/aare/ClusterVector.py @@ -7,16 +7,33 @@ from .ClusterFinder import _get_class def ClusterVector(cluster_size=(3,3), dtype = np.int32): """ - Factory function to create a ClusterVector object. Provides a cleaner syntax for - the templated ClusterVector in C++. + Create an empty ClusterVector for a supported cluster size and pixel dtype. - .. code-block:: python + Parameters + ---------- + cluster_size : tuple[int, int], default=(3, 3) + Cluster dimensions in the x and y directions. + dtype : numpy.dtype, default=numpy.int32 + Pixel storage type. The cluster size and dtype combination must have a + compiled binding. - from aare import ClusterVector - - ClusterVector(cluster_size=(3,3), dtype=np.float64) + Returns + ------- + ClusterVector + An empty vector with frame number 0 and space reserved for at least + 1024 clusters. + + Raises + ------ + ValueError + If the requested size and dtype combination is unavailable. + + Examples + -------- + >>> import numpy as np + >>> from aare import ClusterVector + >>> clusters = ClusterVector(cluster_size=(3, 3), dtype=np.float64) """ cls = _get_class("ClusterVector", cluster_size, dtype) return cls() - diff --git a/python/src/bind_Cluster.hpp b/python/src/bind_Cluster.hpp index 5416d04f..14384125 100644 --- a/python/src/bind_Cluster.hpp +++ b/python/src/bind_Cluster.hpp @@ -67,8 +67,16 @@ void define_Cluster(py::module &m, const std::string &typestr) { return py::make_tuple(max_sum.sum, static_cast(max_sum.index)); }, - R"(calculates sum of 2x2 subcluster with highest energy and index relative to cluster center 0: top_left, 1: top_right, 2: bottom_left, 3: bottom_right - )"); + R"doc( + Return the highest-sum center-adjacent 2x2 subcluster. + + Returns + ------- + tuple + ``(sum, index)``, where index is 0 for top-left, 1 for + top-right, 2 for bottom-left, or 3 for bottom-right relative to + the cluster center. + )doc"); } template &cl) { return reduce_to_3x3(cl); }, - py::return_value_policy::move, R"(Reduce cluster to 3x3 subcluster)"); + py::return_value_policy::move, py::arg("cluster"), R"doc( + Return the 3x3 block around the cluster's center index. + + Both input dimensions must be at least 3, and at least one must be + greater than 3. + The input coordinates are preserved and output data is stored in + row-major order. + )doc"); } template &cl) { return reduce_to_2x2(cl); }, - py::return_value_policy::move, - R"( - Reduce cluster to 2x2 subcluster by taking the 2x2 subcluster with - the highest photon energy. + py::return_value_policy::move, py::arg("cluster"), R"doc( + Return the highest-sum center-adjacent 2x2 block. - RETURN: - - reduced cluster (cluster is filled in row major ordering starting at the top left. Thus for a max subcluster in the top left corner the photon hit is at the fourth position.) - - )"); + The input coordinates are preserved and output data is stored in + row-major order. + )doc"); } -#pragma GCC diagnostic pop \ No newline at end of file +#pragma GCC diagnostic pop diff --git a/python/src/bind_ClusterVector.hpp b/python/src/bind_ClusterVector.hpp index 8c863183..095c3f8a 100644 --- a/python/src/bind_ClusterVector.hpp +++ b/python/src/bind_ClusterVector.hpp @@ -29,37 +29,76 @@ void define_ClusterVector(py::module &m, const std::string &typestr) { py::class_, void>>( - m, class_name.c_str(), + m, class_name.c_str(), R"doc( + A contiguous, move-only container of fixed-size clusters. + + The class supports the Python buffer protocol, so ``numpy.array`` can + either copy its data or create a zero-copy view. A zero-copy view is + valid only while the ClusterVector's underlying allocation and size + remain unchanged. + )doc", py::buffer_protocol()) - .def(py::init()) // TODO change!!! + .def(py::init(), R"doc( + Create an empty ClusterVector with frame number 0 and space reserved + for at least 1024 clusters. + )doc") .def( "__call__", - [](ClusterVector &self, py::array_t mask) { + [](ClusterVector &self, + py::array_t mask) { + if (mask.ndim() != 1) { + throw py::value_error("Mask must be one-dimensional"); + } return self(make_view_1d(mask)); }, - py::arg("mask"), R"( - Create a copy of the clustervector and apply a boolean mask to the ClusterVector. + py::arg("mask").noconvert(), R"doc( + Return a filtered copy of this ClusterVector. Parameters ---------- + mask : numpy.ndarray + One-dimensional, writable, C-contiguous array with dtype + ``numpy.bool_`` and one element per cluster. - mask : 1d boolean numpy array - Mask to apply to the ClusterVector. Must be the same length as the number of clusters in the ClusterVector. + Returns + ------- + ClusterVector + Selected clusters in their original order. The frame number is + preserved. + )doc") - )") + .def( + "push_back", + [](ClusterVector &self, const ClusterType &cluster) { + self.push_back(cluster); + }, + py::arg("cluster"), R"doc( + Append one cluster. - .def("push_back", - [](ClusterVector &self, const ClusterType &cluster) { - self.push_back(cluster); - }) + Notes + ----- + Do not call this method while a zero-copy NumPy view of the + ClusterVector exists. Reallocation invalidates the view, while an + append without reallocation leaves its shape unchanged. + )doc") - .def("sum", - [](ClusterVector &self) { - auto *vec = new std::vector(self.sum()); - return return_vector(vec); - }) + .def( + "sum", + [](ClusterVector &self) { + auto *vec = new std::vector(self.sum()); + return return_vector(vec); + }, + R"doc( + Return the sum of all pixels in each cluster. + + Returns + ------- + numpy.ndarray + One value per cluster, in container order and with the cluster + pixel dtype. + )doc") .def( "sum_2x2", [](ClusterVector &self) { @@ -68,24 +107,45 @@ void define_ClusterVector(py::module &m, const std::string &typestr) { return return_vector(vec); }, - R"(calculates sum of 2x2 subcluster with highest energy and index relative to cluster center 0: top_left, 1: top_right, 2: bottom_left, 3: bottom_right - )") - .def_property_readonly("size", &ClusterVector::size) - .def("empty", &ClusterVector::empty) - .def("item_size", &ClusterVector::item_size) - .def_property_readonly("fmt", - [typestr](ClusterVector &self) { - return fmt_format; - }) + R"doc( + Return the highest-sum center-adjacent 2x2 subcluster for each + cluster. + + Returns + ------- + numpy.ndarray + Structured array with ``sum`` and ``index`` fields. Indices are + 0 for top-left, 1 for top-right, 2 for bottom-left, and 3 for + bottom-right, relative to the cluster center. + )doc") + .def_property_readonly("size", &ClusterVector::size, + "Number of stored clusters.") + .def("empty", &ClusterVector::empty, + "Return True when no clusters are stored.") + .def("item_size", &ClusterVector::item_size, + "Return the size in bytes of one stored cluster, including " + "padding.") + .def_property_readonly( + "fmt", + [typestr](ClusterVector &self) { + return fmt_format; + }, + "PEP 3118 format string for one stored cluster.") .def_property_readonly("cluster_size_x", - &ClusterVector::cluster_size_x) + &ClusterVector::cluster_size_x, + "Cluster size in the x dimension.") .def_property_readonly("cluster_size_y", - &ClusterVector::cluster_size_y) + &ClusterVector::cluster_size_y, + "Cluster size in the y dimension.") .def_property_readonly("capacity", - &ClusterVector::capacity) + &ClusterVector::capacity, + "Number of clusters that fit without " + "reallocation.") .def_property("frame_number", &ClusterVector::frame_number, - &ClusterVector::set_frame_number) + &ClusterVector::set_frame_number, + "Signed 32-bit frame number; 0 can indicate clusters " + "from multiple frames.") .def_buffer( [typestr](ClusterVector &self) -> py::buffer_info { return py::buffer_info( @@ -99,31 +159,38 @@ void define_ClusterVector(py::module &m, const std::string &typestr) { }); // Free functions using ClusterVector - m.def("hitmap", - [](std::array image_size, ClusterVector &cv) { - // Create a numpy array to hold the hitmap - // The shape of the array is (image_size[0], image_size[1]) - // note that the python array is passed as [row, col] which - // is the opposite of the clusters [x,y] - py::array_t hitmap(image_size); - auto r = hitmap.mutable_unchecked<2>(); + m.def( + "hitmap", + [](std::array image_size, ClusterVector &cv) { + // Create a numpy array to hold the hitmap + // The shape of the array is (image_size[0], image_size[1]) + // note that the python array is passed as [row, col] which + // is the opposite of the clusters [x,y] + py::array_t hitmap(image_size); + auto r = hitmap.mutable_unchecked<2>(); - // Initialize hitmap to 0 - for (py::ssize_t i = 0; i < r.shape(0); i++) - for (py::ssize_t j = 0; j < r.shape(1); j++) - r(i, j) = 0; + // Initialize hitmap to 0 + for (py::ssize_t i = 0; i < r.shape(0); i++) + for (py::ssize_t j = 0; j < r.shape(1); j++) + r(i, j) = 0; - // Loop over the clusters and increment the hitmap - // Skip out of bound clusters - for (const auto &cluster : cv) { - auto x = cluster.x; - auto y = cluster.y; - if (x < image_size[1] && y < image_size[0]) - r(cluster.y, cluster.x) += 1; - } + // Loop over the clusters and increment the hitmap + // Skip out of bound clusters + for (const auto &cluster : cv) { + auto x = cluster.x; + auto y = cluster.y; + if (x < image_size[1] && y < image_size[0]) + r(cluster.y, cluster.x) += 1; + } - return hitmap; - }); + return hitmap; + }, + py::arg("image_size"), py::arg("clusters"), R"doc( + Count cluster centers into an ``int32`` image whose shape is given by + ``image_size`` as ``(rows, columns)``. Element ``[y, x]`` contains the + number of cluster centers at that coordinate. Out-of-bounds centers are + ignored. + )doc"); } template >( reduce_to_2x2(cv)); }, - R"( - Reduce cluster to 2x2 subcluster by taking the 2x2 subcluster with - the highest photon energy. - - Parameters - - cv : ClusterVector (clusters are filled in row-major ordering starting at the top left. Thus for a max subcluster in the top left corner the photon hit is at the fourth position.) - - )", + R"doc( + Reduce every cluster to its highest-sum center-adjacent 2x2 block. + Returns a new ClusterVector; cluster order, coordinates, and frame + number are preserved. Input pixel data is interpreted in row-major + order. + )doc", py::arg("clustervector")); } @@ -159,13 +223,13 @@ void define_3x3_reduction(py::module &m) { return new ClusterVector>( reduce_to_3x3(cv)); }, - R"( - Reduce cluster to 3x3 subcluster - - Parameters - - cv : ClusterVector - )", + R"doc( + Reduce every cluster to the 3x3 block around its center index. + Both input dimensions must be at least 3, and at least one must be + greater than 3. + Returns a new ClusterVector; cluster order, coordinates, and frame + number are preserved. + )doc", py::arg("clustervector")); } diff --git a/python/src/module.cpp b/python/src/module.cpp index d53baf4f..140eb958 100644 --- a/python/src/module.cpp +++ b/python/src/module.cpp @@ -128,9 +128,6 @@ PYBIND11_MODULE(_aare, m) { DEFINE_BINDINGS_CLUSTERFINDER(double, 9, 9, uint16_t, d); DEFINE_BINDINGS_CLUSTERFINDER(float, 9, 9, uint16_t, f); - define_3x3_reduction(m); - define_3x3_reduction(m); - define_3x3_reduction(m); define_3x3_reduction(m); define_3x3_reduction(m); define_3x3_reduction(m); @@ -141,9 +138,6 @@ PYBIND11_MODULE(_aare, m) { define_3x3_reduction(m); define_3x3_reduction(m); - reduce_to_3x3(m); - reduce_to_3x3(m); - reduce_to_3x3(m); reduce_to_3x3(m); reduce_to_3x3(m); reduce_to_3x3(m); diff --git a/python/tests/test_ClusterVector.py b/python/tests/test_ClusterVector.py index 30f934fa..8b077dc0 100644 --- a/python/tests/test_ClusterVector.py +++ b/python/tests/test_ClusterVector.py @@ -83,12 +83,15 @@ def test_make_a_hitmap_from_cluster_vector(): def test_2x2_reduction(): cv = ClusterVector((3,3)) + cv.frame_number = -135 cv.push_back(_aare.Cluster3x3i(5, 5, np.array([1, 1, 1, 2, 3, 1, 2, 2, 1], dtype=np.int32))) cv.push_back(_aare.Cluster3x3i(5, 5, np.array([2, 2, 1, 2, 3, 1, 1, 1, 1], dtype=np.int32))) - reduced_cv = np.array(_aare.reduce_to_2x2(cv), copy=False) + reduced = _aare.reduce_to_2x2(cv) + reduced_cv = np.array(reduced, copy=False) + assert reduced.frame_number == cv.frame_number assert reduced_cv.size == 2 assert reduced_cv[0]["x"] == 5 assert reduced_cv[0]["y"] == 5 @@ -100,14 +103,17 @@ def test_2x2_reduction(): def test_3x3_reduction(): cv = _aare.ClusterVector_Cluster5x5d() + cv.frame_number = 246 cv.push_back(_aare.Cluster5x5d(5,5,np.array([1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 2.0, 2.0, 3.0, 1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0], dtype=np.double))) cv.push_back(_aare.Cluster5x5d(5,5,np.array([1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 2.0, 2.0, 3.0, 1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0], dtype=np.double))) - reduced_cv = np.array(_aare.reduce_to_3x3(cv), copy=False) + reduced = _aare.reduce_to_3x3(cv) + reduced_cv = np.array(reduced, copy=False) + assert reduced.frame_number == cv.frame_number assert reduced_cv.size == 2 assert reduced_cv[0]["x"] == 5 assert reduced_cv[0]["y"] == 5 @@ -130,3 +136,26 @@ def test_masking(): assert cv_masked_array[0]["x"] == 1 assert cv_masked_array[0]["y"] == 2 assert (cv_masked_array[0]["data"] == np.ones((3,3),dtype=np.int32)).all() + + +def test_masking_requires_c_contiguous_array(): + cv = _aare.ClusterVector_Cluster3x3i() + cv.push_back(_aare.Cluster3x3i(1, 2, np.ones(9, dtype=np.int32))) + cv.push_back(_aare.Cluster3x3i(3, 4, np.ones(9, dtype=np.int32))) + + mask = np.array([True, False, True, False], dtype=bool)[::2] + assert not mask.flags.c_contiguous + + with pytest.raises(TypeError): + cv(mask) + + +def test_masking_requires_one_dimension(): + cv = _aare.ClusterVector_Cluster3x3i() + cv.push_back(_aare.Cluster3x3i(1, 2, np.ones(9, dtype=np.int32))) + cv.push_back(_aare.Cluster3x3i(3, 4, np.ones(9, dtype=np.int32))) + + mask = np.array([[True, False]], dtype=bool) + + with pytest.raises(ValueError, match="one-dimensional"): + cv(mask) diff --git a/src/Cluster.test.cpp b/src/Cluster.test.cpp index 4919b859..ddcd2c4b 100644 --- a/src/Cluster.test.cpp +++ b/src/Cluster.test.cpp @@ -24,8 +24,8 @@ TEST_CASE("Test sum of Cluster", "[cluster]") { using ClusterTypes = std::variant, Cluster, Cluster, Cluster>; -using ClusterTypesLargerThan2x2 = - std::variant, Cluster, Cluster>; +using ReducibleTo3x3ClusterTypes = + std::variant, Cluster, Cluster>; TEST_CASE("Test reduce to 2x2 Cluster", "[cluster]") { auto [cluster, expected_reduced_cluster] = GENERATE( @@ -68,14 +68,15 @@ TEST_CASE("Test reduce to 2x2 Cluster", "[cluster]") { TEST_CASE("Test reduce to 3x3 Cluster", "[cluster]") { auto [cluster, expected_reduced_cluster] = GENERATE( - std::make_tuple(ClusterTypesLargerThan2x2{Cluster{ - 5, 5, {1, 1, 1, 1, 3, 1, 1, 1, 1}}}, - Cluster{5, 5, {1, 1, 1, 1, 3, 1, 1, 1, 1}}), std::make_tuple( - ClusterTypesLargerThan2x2{Cluster{ - 5, 5, {2, 2, 1, 1, 2, 2, 1, 1, 1, 1, 3, 1, 1, 1, 1, 1}}}, - Cluster{5, 5, {2, 1, 1, 1, 3, 1, 1, 1, 1}}), - std::make_tuple(ClusterTypesLargerThan2x2{Cluster{ + ReducibleTo3x3ClusterTypes{Cluster{ + 5, 5, {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}}, + Cluster{5, 5, {3, 4, 5, 6, 7, 8, 9, 10, 11}}), + std::make_tuple( + ReducibleTo3x3ClusterTypes{Cluster{ + 5, 5, {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}}, + Cluster{5, 5, {1, 2, 3, 5, 6, 7, 9, 10, 11}}), + std::make_tuple(ReducibleTo3x3ClusterTypes{Cluster{ 5, 5, {1, 1, 1, 1, 1, 1, 2, 1, 1, 1, 2, 2, 3, 1, 1, 1, 2, 1, 1, 1, 1, 1, 1, 1, 1}}}, Cluster{5, 5, {2, 1, 1, 2, 3, 1, 2, 1, 1}})); @@ -89,4 +90,4 @@ TEST_CASE("Test reduce to 3x3 Cluster", "[cluster]") { CHECK(std::equal(reduced_cluster.data.begin(), reduced_cluster.data.begin() + 9, expected_reduced_cluster.data.begin())); -} \ No newline at end of file +} diff --git a/src/ClusterVector.test.cpp b/src/ClusterVector.test.cpp index 2c098e14..b4e3abf6 100644 --- a/src/ClusterVector.test.cpp +++ b/src/ClusterVector.test.cpp @@ -22,27 +22,79 @@ TEST_CASE("After pushing back one element the ClusterVector is not empty") { REQUIRE(!cv.empty()); } -TEST_CASE("ClusterVector move constructor preserves contents") { - ClusterVector source(4, 42); +TEST_CASE("Filtering a ClusterVector selects the masked elements") { + ClusterVector source(8, 123); source.push_back(C1{1, 2, {3, 4, 5, 6}}); + source.push_back(C1{7, 8, {9, 10, 11, 12}}); + source.push_back(C1{13, 14, {15, 16, 17, 18}}); + source.push_back(C1{19, 20, {21, 22, 23, 24}}); - ClusterVector moved(std::move(source)); + SECTION("Some elements are selected") { + std::array mask{true, false, false, true}; - REQUIRE(moved.size() == 1); - CHECK(moved.frame_number() == 42); - CHECK(moved[0].data[3] == 6); + auto filtered = source(aare::NDView{mask.data(), {4}}); + + CHECK(filtered.size() == 2); + CHECK(filtered.frame_number() == 123); + CHECK(filtered[0].x == 1); + CHECK(filtered[1].x == 19); + } + + SECTION("No elements are selected") { + std::array mask{false, false, false, false}; + + auto filtered = source(aare::NDView{mask.data(), {4}}); + + CHECK(filtered.empty()); + CHECK(filtered.frame_number() == 123); + } + + SECTION("An empty vector accepts a default empty mask") { + ClusterVector empty_source(0, -123); + + auto filtered = empty_source(aare::NDView{}); + + CHECK(filtered.empty()); + CHECK(filtered.frame_number() == -123); + } } -TEST_CASE("ClusterVector move assignment preserves contents") { - ClusterVector source(4, 42); +TEST_CASE("Move constructing a ClusterVector transfers its storage") { + ClusterVector source(4, 123); source.push_back(C1{1, 2, {3, 4, 5, 6}}); - ClusterVector moved(1); + source.push_back(C1{7, 8, {9, 10, 11, 12}}); - moved = std::move(source); + const auto *source_data = source.data(); + const auto source_capacity = source.capacity(); - REQUIRE(moved.size() == 1); - CHECK(moved.frame_number() == 42); - CHECK(moved[0].data[3] == 6); + ClusterVector destination(std::move(source)); + + CHECK(destination.data() == source_data); + CHECK(destination.capacity() == source_capacity); + CHECK(destination.size() == 2); + CHECK(destination.frame_number() == 123); + CHECK(destination[0].x == 1); + CHECK(destination[1].x == 7); +} + +TEST_CASE("Move assigning a ClusterVector transfers its storage") { + ClusterVector source(4, 123); + source.push_back(C1{1, 2, {3, 4, 5, 6}}); + source.push_back(C1{7, 8, {9, 10, 11, 12}}); + + const auto *source_data = source.data(); + const auto source_capacity = source.capacity(); + + ClusterVector destination(2, 456); + destination.push_back(C1{13, 14, {15, 16, 17, 18}}); + destination = std::move(source); + + CHECK(destination.data() == source_data); + CHECK(destination.capacity() == source_capacity); + CHECK(destination.size() == 2); + CHECK(destination.frame_number() == 123); + CHECK(destination[0].x == 1); + CHECK(destination[1].x == 7); } TEST_CASE("item_size return the size of the cluster stored") { @@ -256,6 +308,28 @@ TEST_CASE("Concatenate two cluster vectors where we need to allocate") { REQUIRE(ptr[3].y == 17); } +TEST_CASE("Reducing a ClusterVector preserves its frame number") { + SECTION("Reduce to 2x2") { + ClusterVector> source(1, -135); + source.push_back(Cluster{}); + + auto reduced = aare::reduce_to_2x2(source); + + CHECK(reduced.size() == source.size()); + CHECK(reduced.frame_number() == source.frame_number()); + } + + SECTION("Reduce to 3x3") { + ClusterVector> source(1, -246); + source.push_back(Cluster{}); + + auto reduced = aare::reduce_to_3x3(source); + + CHECK(reduced.size() == source.size()); + CHECK(reduced.frame_number() == source.frame_number()); + } +} + struct ClusterTestData { uint8_t ClusterSizeX; uint8_t ClusterSizeY;