From 8b0eee1e66dd7f6273bb4c7ceb7cd3d67a15f52a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= Date: Wed, 9 Apr 2025 17:54:55 +0200 Subject: [PATCH] fixed warnings and removed ambiguous read_frame (#154) Fixed warnings: - unused variable in Interpolator - Narrowing conversions uint64-->int64 Removed an ambiguous function from JungfrauDataFile - NDarry read_frame(header&=nullptr) - Frame read_frame() NDArray and NDView size() is now signed --- include/aare/JungfrauDataFile.hpp | 8 +------- include/aare/NDArray.hpp | 2 +- include/aare/NDView.hpp | 4 ++-- include/aare/VarClusterFinder.hpp | 4 ++-- src/Fit.cpp | 8 ++++---- src/Interpolator.cpp | 9 ++------- src/JungfrauDataFile.cpp | 20 ++++++++------------ src/JungfrauDataFile.test.cpp | 24 ++++++++++++++++++++++-- src/NDArray.test.cpp | 4 ++-- src/algorithm.test.cpp | 12 ++++++------ 10 files changed, 50 insertions(+), 45 deletions(-) diff --git a/include/aare/JungfrauDataFile.hpp b/include/aare/JungfrauDataFile.hpp index bba5403..9b1bc48 100644 --- a/include/aare/JungfrauDataFile.hpp +++ b/include/aare/JungfrauDataFile.hpp @@ -49,6 +49,7 @@ class JungfrauDataFile : public FileInterface { size_t total_frames() const override; size_t rows() const override; size_t cols() const override; + std::array shape() const; size_t n_files() const; //!< get the number of files in the series. // Extra functions needed for FileInterface @@ -81,13 +82,6 @@ class JungfrauDataFile : public FileInterface { */ void read_into(NDArray* image, JungfrauDataHeader* header = nullptr); - /** - * @brief Read a single frame from the file. Allocated a new NDArray for the output data - * @param header pointer to a JungfrauDataHeader or nullptr to skip header) - * @return NDArray with the image data - */ - NDArray read_frame(JungfrauDataHeader* header = nullptr); - JungfrauDataHeader read_header(); std::filesystem::path current_file() const { return fpath(m_current_file_index+m_offset); } diff --git a/include/aare/NDArray.hpp b/include/aare/NDArray.hpp index 45d3a83..ceb1e0b 100644 --- a/include/aare/NDArray.hpp +++ b/include/aare/NDArray.hpp @@ -194,7 +194,7 @@ class NDArray : public ArrayExpr, Ndim> { T *data() { return data_; } std::byte *buffer() { return reinterpret_cast(data_); } - size_t size() const { return size_; } + ssize_t size() const { return static_cast(size_); } size_t total_bytes() const { return size_ * sizeof(T); } std::array shape() const noexcept { return shape_; } int64_t shape(int64_t i) const noexcept { return shape_[i]; } diff --git a/include/aare/NDView.hpp b/include/aare/NDView.hpp index f53f758..55b442b 100644 --- a/include/aare/NDView.hpp +++ b/include/aare/NDView.hpp @@ -71,7 +71,7 @@ template class NDView : public ArrayExpr(size_); } size_t total_bytes() const { return size_ * sizeof(T); } std::array strides() const noexcept { return strides_; } @@ -102,7 +102,7 @@ template class NDView : public ArrayExpr NDView& operator=(const std::array &arr) { - if(size() != arr.size()) + if(size() != static_cast(arr.size())) throw std::runtime_error(LOCATION + "Array and NDView size mismatch"); std::copy(arr.begin(), arr.end(), begin()); return *this; diff --git a/include/aare/VarClusterFinder.hpp b/include/aare/VarClusterFinder.hpp index ea62a9d..161941a 100644 --- a/include/aare/VarClusterFinder.hpp +++ b/include/aare/VarClusterFinder.hpp @@ -226,7 +226,7 @@ template void VarClusterFinder::single_pass(NDView img) { template void VarClusterFinder::first_pass() { - for (size_t i = 0; i < original_.size(); ++i) { + for (ssize_t i = 0; i < original_.size(); ++i) { if (use_noise_map) threshold_ = 5 * noiseMap(i); binary_(i) = (original_(i) > threshold_); @@ -250,7 +250,7 @@ template void VarClusterFinder::first_pass() { template void VarClusterFinder::second_pass() { - for (size_t i = 0; i != labeled_.size(); ++i) { + for (ssize_t i = 0; i != labeled_.size(); ++i) { auto cl = labeled_(i); if (cl != 0) { auto it = child.find(cl); diff --git a/src/Fit.cpp b/src/Fit.cpp index 3001efd..9126109 100644 --- a/src/Fit.cpp +++ b/src/Fit.cpp @@ -18,7 +18,7 @@ double gaus(const double x, const double *par) { NDArray gaus(NDView x, NDView par) { NDArray y({x.shape(0)}, 0); - for (size_t i = 0; i < x.size(); i++) { + for (ssize_t i = 0; i < x.size(); i++) { y(i) = gaus(x(i), par.data()); } return y; @@ -28,7 +28,7 @@ double pol1(const double x, const double *par) { return par[0] * x + par[1]; } NDArray pol1(NDView x, NDView par) { NDArray y({x.shape()}, 0); - for (size_t i = 0; i < x.size(); i++) { + for (ssize_t i = 0; i < x.size(); i++) { y(i) = pol1(x(i), par.data()); } return y; @@ -153,7 +153,7 @@ void fit_gaus(NDView x, NDView y, NDView y_err, // Calculate chi2 chi2 = 0; - for (size_t i = 0; i < y.size(); i++) { + for (ssize_t i = 0; i < y.size(); i++) { chi2 += std::pow((y(i) - func::gaus(x(i), par_out.data())) / y_err(i), 2); } } @@ -205,7 +205,7 @@ void fit_pol1(NDView x, NDView y, NDView y_err, // Calculate chi2 chi2 = 0; - for (size_t i = 0; i < y.size(); i++) { + for (ssize_t i = 0; i < y.size(); i++) { chi2 += std::pow((y(i) - func::pol1(x(i), par_out.data())) / y_err(i), 2); } } diff --git a/src/Interpolator.cpp b/src/Interpolator.cpp index 7f82533..7034a83 100644 --- a/src/Interpolator.cpp +++ b/src/Interpolator.cpp @@ -68,19 +68,14 @@ std::vector Interpolator::interpolate(const ClusterVector& clus photon.y = cluster.y; photon.energy = eta.sum; - // auto ie = nearest_index(m_energy_bins, photon.energy)-1; - // auto ix = nearest_index(m_etabinsx, eta.x)-1; - // auto iy = nearest_index(m_etabinsy, eta.y)-1; + //Finding the index of the last element that is smaller //should work fine as long as we have many bins auto ie = last_smaller(m_energy_bins, photon.energy); auto ix = last_smaller(m_etabinsx, eta.x); auto iy = last_smaller(m_etabinsy, eta.y); - - // fmt::print("ex: {}, ix: {}, iy: {}\n", ie, ix, iy); - double dX, dY; - int ex, ey; + double dX{}, dY{}; // cBottomLeft = 0, // cBottomRight = 1, // cTopLeft = 2, diff --git a/src/JungfrauDataFile.cpp b/src/JungfrauDataFile.cpp index 6e1ccd6..8f1f904 100644 --- a/src/JungfrauDataFile.cpp +++ b/src/JungfrauDataFile.cpp @@ -37,8 +37,9 @@ Frame JungfrauDataFile::read_frame(size_t frame_number){ std::vector JungfrauDataFile::read_n(size_t n_frames) { std::vector frames; - throw std::runtime_error(LOCATION + - "Not implemented yet"); + for(size_t i = 0; i < n_frames; ++i){ + frames.push_back(read_frame()); + } return frames; } @@ -54,6 +55,10 @@ size_t JungfrauDataFile::frame_number(size_t frame_index) { return read_header().framenum; } +std::array JungfrauDataFile::shape() const { + return {static_cast(rows()), static_cast(cols())}; +} + DetectorType JungfrauDataFile::detector_type() const { return DetectorType::Jungfrau; } std::string JungfrauDataFile::base_name() const { return m_base_name; } @@ -198,22 +203,13 @@ void JungfrauDataFile::read_into(std::byte *image_buf, size_t n_frames, } void JungfrauDataFile::read_into(NDArray* image, JungfrauDataHeader* header) { - if(!(rows() == image->shape(0) && cols() == image->shape(1))){ + if(image->shape()!=shape()){ throw std::runtime_error(LOCATION + "Image shape does not match file size: " + std::to_string(rows()) + "x" + std::to_string(cols())); } read_into(reinterpret_cast(image->data()), header); } -NDArray JungfrauDataFile::read_frame(JungfrauDataHeader* header) { - Shape<2> shape{rows(), cols()}; - NDArray image(shape); - - read_into(reinterpret_cast(image.data()), - header); - - return image; -} JungfrauDataHeader JungfrauDataFile::read_header() { JungfrauDataHeader header; diff --git a/src/JungfrauDataFile.test.cpp b/src/JungfrauDataFile.test.cpp index 626a318..ce51168 100644 --- a/src/JungfrauDataFile.test.cpp +++ b/src/JungfrauDataFile.test.cpp @@ -28,7 +28,8 @@ TEST_CASE("Open a Jungfrau data file", "[.files]") { //Check that the frame number and buch id is read correctly for (size_t i = 0; i < 24; ++i) { JungfrauDataHeader header; - auto image = f.read_frame(&header); + aare::NDArray image(f.shape()); + f.read_into(&image, &header); REQUIRE(header.framenum == i + 1); REQUIRE(header.bunchid == (i + 1) * (i + 1)); REQUIRE(image.shape(0) == 512); @@ -58,7 +59,8 @@ TEST_CASE("Seek in a JungfrauDataFile", "[.files]"){ REQUIRE(h3.framenum == 59+1); JungfrauDataHeader h4; - auto image = f.read_frame(&h4); + aare::NDArray image(f.shape()); + f.read_into(&image, &h4); REQUIRE(h4.framenum == 59+1); //now we should be on the next frame @@ -91,4 +93,22 @@ TEST_CASE("Open a Jungfrau data file with non zero file index", "[.files]"){ REQUIRE(f.current_file().stem() == "AldoJF65k_000003"); +} + +TEST_CASE("Read into throws if size doesn't match", "[.files]"){ + auto fpath = test_data_path() / "dat" / "AldoJF65k_000000.dat"; + REQUIRE(std::filesystem::exists(fpath)); + + JungfrauDataFile f(fpath); + + aare::NDArray image({39, 85}); + JungfrauDataHeader header; + + REQUIRE_THROWS(f.read_into(&image, &header)); + REQUIRE_THROWS(f.read_into(&image, nullptr)); + REQUIRE_THROWS(f.read_into(&image)); + + REQUIRE(f.tell() == 0); + + } \ No newline at end of file diff --git a/src/NDArray.test.cpp b/src/NDArray.test.cpp index eff3e2c..c37a285 100644 --- a/src/NDArray.test.cpp +++ b/src/NDArray.test.cpp @@ -183,14 +183,14 @@ TEST_CASE("Size and shape matches") { int64_t h = 75; std::array shape{w, h}; NDArray a{shape}; - REQUIRE(a.size() == static_cast(w * h)); + REQUIRE(a.size() == w * h); REQUIRE(a.shape() == shape); } TEST_CASE("Initial value matches for all elements") { double v = 4.35; NDArray a{{5, 5}, v}; - for (uint32_t i = 0; i < a.size(); ++i) { + for (int i = 0; i < a.size(); ++i) { REQUIRE(a(i) == v); } } diff --git a/src/algorithm.test.cpp b/src/algorithm.test.cpp index e2ae8fa..79541a1 100644 --- a/src/algorithm.test.cpp +++ b/src/algorithm.test.cpp @@ -6,7 +6,7 @@ TEST_CASE("Find the closed index in a 1D array", "[algorithm]") { aare::NDArray arr({5}); - for (size_t i = 0; i < arr.size(); i++) { + for (ssize_t i = 0; i < arr.size(); i++) { arr[i] = i; } // arr 0, 1, 2, 3, 4 @@ -19,7 +19,7 @@ TEST_CASE("Find the closed index in a 1D array", "[algorithm]") { TEST_CASE("Passing integers to nearest_index works", "[algorithm]"){ aare::NDArray arr({5}); - for (size_t i = 0; i < arr.size(); i++) { + for (ssize_t i = 0; i < arr.size(); i++) { arr[i] = i; } // arr 0, 1, 2, 3, 4 @@ -62,7 +62,7 @@ TEST_CASE("nearest index when there is no different uses the first element also TEST_CASE("last smaller", "[algorithm]"){ aare::NDArray arr({5}); - for (size_t i = 0; i < arr.size(); i++) { + for (ssize_t i = 0; i < arr.size(); i++) { arr[i] = i; } // arr 0, 1, 2, 3, 4 @@ -74,7 +74,7 @@ TEST_CASE("last smaller", "[algorithm]"){ TEST_CASE("returns last bin strictly smaller", "[algorithm]"){ aare::NDArray arr({5}); - for (size_t i = 0; i < arr.size(); i++) { + for (ssize_t i = 0; i < arr.size(); i++) { arr[i] = i; } // arr 0, 1, 2, 3, 4 @@ -84,7 +84,7 @@ TEST_CASE("returns last bin strictly smaller", "[algorithm]"){ TEST_CASE("last_smaller with all elements smaller returns last element", "[algorithm]"){ aare::NDArray arr({5}); - for (size_t i = 0; i < arr.size(); i++) { + for (ssize_t i = 0; i < arr.size(); i++) { arr[i] = i; } // arr 0, 1, 2, 3, 4 @@ -93,7 +93,7 @@ TEST_CASE("last_smaller with all elements smaller returns last element", "[algor TEST_CASE("last_smaller with all elements bigger returns first element", "[algorithm]"){ aare::NDArray arr({5}); - for (size_t i = 0; i < arr.size(); i++) { + for (ssize_t i = 0; i < arr.size(); i++) { arr[i] = i; } // arr 0, 1, 2, 3, 4