From f06e722dce1a881eac25302814c8fa96d83e77c9 Mon Sep 17 00:00:00 2001 From: mazzol_a Date: Fri, 25 Apr 2025 11:38:51 +0200 Subject: [PATCH] changes from PR review --- include/aare/CalculateEta.hpp | 6 +-- include/aare/ClusterVector.hpp | 19 ++++----- include/aare/GainMap.hpp | 3 +- include/aare/Interpolator.hpp | 6 +-- src/ClusterFile.test.cpp | 70 ++++++++++++++++------------------ src/ClusterVector.test.cpp | 2 +- 6 files changed, 50 insertions(+), 56 deletions(-) diff --git a/include/aare/CalculateEta.hpp b/include/aare/CalculateEta.hpp index 289e8bc..37bdf00 100644 --- a/include/aare/CalculateEta.hpp +++ b/include/aare/CalculateEta.hpp @@ -40,8 +40,8 @@ template calculate_eta2(const ClusterVector &clusters) { NDArray eta2({static_cast(clusters.size()), 2}); - for (size_t i = 0; i < clusters.size(); i++) { - auto e = calculate_eta2(clusters.at(i)); + for (const ClusterType &cluster : clusters) { + auto e = calculate_eta2(cluster); eta2(i, 0) = e.x; eta2(i, 1) = e.y; } @@ -122,7 +122,7 @@ calculate_eta2(const Cluster &cl) { return eta; } -// Dont get why this is correct - photon center should be top right corner +// TODO! Look up eta2 calculation - photon center should be top right corner template Eta2 calculate_eta2(const Cluster &cl) { Eta2 eta{}; diff --git a/include/aare/ClusterVector.hpp b/include/aare/ClusterVector.hpp index e91cb6d..5630278 100644 --- a/include/aare/ClusterVector.hpp +++ b/include/aare/ClusterVector.hpp @@ -76,9 +76,9 @@ class ClusterVector> { std::vector sum() { std::vector sums(m_data.size()); - for (size_t i = 0; i < m_data.size(); i++) { - sums[i] = at(i).sum(); - } + std::transform(m_data.begin(), m_data.end(), sums.begin(), + [](const T &cluster) { return cluster.sum(); }); + return sums; } @@ -86,13 +86,14 @@ class ClusterVector> { * @brief Sum the pixels in the 2x2 subcluster with the biggest pixel sum in * each cluster * @return std::vector vector of sums for each cluster - */ //TODO if underlying container is a vector use std::for_each + */ std::vector sum_2x2() { std::vector sums_2x2(m_data.size()); - for (size_t i = 0; i < m_data.size(); i++) { - sums_2x2[i] = at(i).max_sum_2x2().first; - } + std::transform( + m_data.begin(), m_data.end(), sums_2x2.begin(), + [](const T &cluster) { return cluster.max_sum_2x2().first; }); + return sums_2x2; } @@ -149,9 +150,9 @@ class ClusterVector> { * @brief Return a reference to the i-th cluster casted to type V * @tparam V type of the cluster */ - ClusterType &at(size_t i) { return m_data[i]; } + ClusterType &operator[](size_t i) { return m_data[i]; } - const ClusterType &at(size_t i) const { return m_data[i]; } + const ClusterType &operator[](size_t i) const { return m_data[i]; } /** * @brief Return the frame number of the clusters. 0 is used to indicate diff --git a/include/aare/GainMap.hpp b/include/aare/GainMap.hpp index 621cc9f..23ed467 100644 --- a/include/aare/GainMap.hpp +++ b/include/aare/GainMap.hpp @@ -41,8 +41,7 @@ class GainMap { int64_t index_cluster_center_x = ClusterSizeX / 2; int64_t index_cluster_center_y = ClusterSizeY / 2; - for (size_t i = 0; i < clustervec.size(); i++) { - auto &cl = clustervec.at(i); + for (T &cl : clustervec) { if (cl.x > 0 && cl.y > 0 && cl.x < m_gain_map.shape(1) - 1 && cl.y < m_gain_map.shape(0) - 1) { diff --git a/include/aare/Interpolator.hpp b/include/aare/Interpolator.hpp index 85ccf29..d2b2322 100644 --- a/include/aare/Interpolator.hpp +++ b/include/aare/Interpolator.hpp @@ -44,9 +44,8 @@ Interpolator::interpolate(const ClusterVector &clusters) { photons.reserve(clusters.size()); if (clusters.cluster_size_x() == 3 || clusters.cluster_size_y() == 3) { - for (size_t i = 0; i < clusters.size(); i++) { + for (const ClusterType &cluster : clusters) { - auto cluster = clusters.at(i); auto eta = calculate_eta2(cluster); Photon photon; @@ -94,8 +93,7 @@ Interpolator::interpolate(const ClusterVector &clusters) { } } else if (clusters.cluster_size_x() == 2 || clusters.cluster_size_y() == 2) { - for (size_t i = 0; i < clusters.size(); i++) { - auto cluster = clusters.at(i); + for (const ClusterType &cluster : clusters) { auto eta = calculate_eta2(cluster); Photon photon; diff --git a/src/ClusterFile.test.cpp b/src/ClusterFile.test.cpp index f688c3d..6254b5d 100644 --- a/src/ClusterFile.test.cpp +++ b/src/ClusterFile.test.cpp @@ -20,11 +20,10 @@ TEST_CASE("Read one frame from a cluster file", "[.files]") { auto clusters = f.read_frame(); CHECK(clusters.size() == 97); CHECK(clusters.frame_number() == 135); - CHECK(clusters.at(0).x == 1); - CHECK(clusters.at(0).y == 200); + CHECK(clusters[0].x == 1); + CHECK(clusters[0].y == 200); int32_t expected_cluster_data[] = {0, 1, 2, 3, 4, 5, 6, 7, 8}; - CHECK(std::equal(std::begin(clusters.at(0).data), - std::end(clusters.at(0).data), + CHECK(std::equal(std::begin(clusters[0].data), std::end(clusters[0].data), std::begin(expected_cluster_data))); } @@ -47,18 +46,17 @@ TEST_CASE("Read one frame using ROI", "[.files]") { // Check that all clusters are within the ROI for (size_t i = 0; i < clusters.size(); i++) { - auto c = clusters.at(i); + auto c = clusters[i]; REQUIRE(c.x >= roi.xmin); REQUIRE(c.x <= roi.xmax); REQUIRE(c.y >= roi.ymin); REQUIRE(c.y <= roi.ymax); } - CHECK(clusters.at(0).x == 1); - CHECK(clusters.at(0).y == 200); + CHECK(clusters[0].x == 1); + CHECK(clusters[0].y == 200); int32_t expected_cluster_data[] = {0, 1, 2, 3, 4, 5, 6, 7, 8}; - CHECK(std::equal(std::begin(clusters.at(0).data), - std::end(clusters.at(0).data), + CHECK(std::equal(std::begin(clusters[0].data), std::end(clusters[0].data), std::begin(expected_cluster_data))); } @@ -175,10 +173,10 @@ TEST_CASE("Read clusters from single frame file", "[.files]") { REQUIRE(clusters.size() == 50); REQUIRE(clusters.frame_number() == 135); int32_t expected_cluster_data[] = {0, 1, 2, 3, 4, 5, 6, 7, 8}; - REQUIRE(clusters.at(0).x == 1); - REQUIRE(clusters.at(0).y == 200); - CHECK(std::equal(std::begin(clusters.at(0).data), - std::end(clusters.at(0).data), + REQUIRE(clusters[0].x == 1); + REQUIRE(clusters[0].y == 200); + CHECK(std::equal(std::begin(clusters[0].data), + std::end(clusters[0].data), std::begin(expected_cluster_data))); } SECTION("Read more clusters than available") { @@ -188,10 +186,10 @@ TEST_CASE("Read clusters from single frame file", "[.files]") { REQUIRE(clusters.size() == 97); REQUIRE(clusters.frame_number() == 135); int32_t expected_cluster_data[] = {0, 1, 2, 3, 4, 5, 6, 7, 8}; - REQUIRE(clusters.at(0).x == 1); - REQUIRE(clusters.at(0).y == 200); - CHECK(std::equal(std::begin(clusters.at(0).data), - std::end(clusters.at(0).data), + REQUIRE(clusters[0].x == 1); + REQUIRE(clusters[0].y == 200); + CHECK(std::equal(std::begin(clusters[0].data), + std::end(clusters[0].data), std::begin(expected_cluster_data))); } SECTION("Read all clusters") { @@ -199,11 +197,11 @@ TEST_CASE("Read clusters from single frame file", "[.files]") { auto clusters = f.read_clusters(97); REQUIRE(clusters.size() == 97); REQUIRE(clusters.frame_number() == 135); - REQUIRE(clusters.at(0).x == 1); - REQUIRE(clusters.at(0).y == 200); + REQUIRE(clusters[0].x == 1); + REQUIRE(clusters[0].y == 200); int32_t expected_cluster_data[] = {0, 1, 2, 3, 4, 5, 6, 7, 8}; - CHECK(std::equal(std::begin(clusters.at(0).data), - std::end(clusters.at(0).data), + CHECK(std::equal(std::begin(clusters[0].data), + std::end(clusters[0].data), std::begin(expected_cluster_data))); } } @@ -225,11 +223,10 @@ TEST_CASE("Read clusters from single frame file with ROI", "[.files]") { CHECK(clusters.size() == 10); CHECK(clusters.frame_number() == 135); - CHECK(clusters.at(0).x == 1); - CHECK(clusters.at(0).y == 200); + CHECK(clusters[0].x == 1); + CHECK(clusters[0].y == 200); int32_t expected_cluster_data[] = {0, 1, 2, 3, 4, 5, 6, 7, 8}; - CHECK(std::equal(std::begin(clusters.at(0).data), - std::end(clusters.at(0).data), + CHECK(std::equal(std::begin(clusters[0].data), std::end(clusters[0].data), std::begin(expected_cluster_data))); } @@ -314,19 +311,19 @@ TEST_CASE("Write cluster with potential padding", "[.files][.ClusterFile]") { CHECK(read_cluster_vector.size() == 2); CHECK(read_cluster_vector.frame_number() == 0); - CHECK(read_cluster_vector.at(0).x == clustervec.at(0).x); - CHECK(read_cluster_vector.at(0).y == clustervec.at(0).y); + CHECK(read_cluster_vector[0].x == clustervec[0].x); + CHECK(read_cluster_vector[0].y == clustervec[0].y); CHECK(std::equal( - clustervec.at(0).data.begin(), clustervec.at(0).data.end(), - read_cluster_vector.at(0).data.begin(), [](double a, double b) { + clustervec[0].data.begin(), clustervec[0].data.end(), + read_cluster_vector[0].data.begin(), [](double a, double b) { return std::abs(a - b) < std::numeric_limits::epsilon(); })); - CHECK(read_cluster_vector.at(1).x == clustervec.at(1).x); - CHECK(read_cluster_vector.at(1).y == clustervec.at(1).y); + CHECK(read_cluster_vector[1].x == clustervec[1].x); + CHECK(read_cluster_vector[1].y == clustervec[1].y); CHECK(std::equal( - clustervec.at(1).data.begin(), clustervec.at(1).data.end(), - read_cluster_vector.at(1).data.begin(), [](double a, double b) { + clustervec[1].data.begin(), clustervec[1].data.end(), + read_cluster_vector[1].data.begin(), [](double a, double b) { return std::abs(a - b) < std::numeric_limits::epsilon(); })); } @@ -346,10 +343,9 @@ TEST_CASE("Read frame and modify cluster data", "[.files][.ClusterFile]") { Cluster{0, 0, {0, 1, 2, 3, 4, 5, 6, 7, 8}}); CHECK(clusters.size() == 98); - CHECK(clusters.at(0).x == 1); - CHECK(clusters.at(0).y == 200); + CHECK(clusters[0].x == 1); + CHECK(clusters[0].y == 200); - CHECK(std::equal(std::begin(clusters.at(0).data), - std::end(clusters.at(0).data), + CHECK(std::equal(std::begin(clusters[0].data), std::end(clusters[0].data), std::begin(expected_cluster_data))); } diff --git a/src/ClusterVector.test.cpp b/src/ClusterVector.test.cpp index 468a707..1214b6b 100644 --- a/src/ClusterVector.test.cpp +++ b/src/ClusterVector.test.cpp @@ -60,7 +60,7 @@ TEST_CASE("ClusterVector 2x2 int32_t capacity 4, push back then read", REQUIRE(cv.size() == 1); REQUIRE(cv.capacity() == 4); - auto c2 = cv.at(0); + auto c2 = cv[0]; // Check that the data is the same REQUIRE(c1.x == c2.x);