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

@@ -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<ssize_t,2> 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<uint16_t>* 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<uint16_t> read_frame(JungfrauDataHeader* header = nullptr);
-
     JungfrauDataHeader read_header();
     std::filesystem::path current_file() const { return fpath(m_current_file_index+m_offset); }
 

include/aare/NDArray.hpp

@@ -194,7 +194,7 @@ class NDArray : public ArrayExpr<NDArray<T, Ndim>, Ndim> {
 
     T *data() { return data_; }
     std::byte *buffer() { return reinterpret_cast<std::byte *>(data_); }
-    size_t size() const { return size_; }
+    ssize_t size() const { return static_cast<ssize_t>(size_); }
     size_t total_bytes() const { return size_ * sizeof(T); }
     std::array<int64_t, Ndim> shape() const noexcept { return shape_; }
     int64_t shape(int64_t i) const noexcept { return shape_[i]; }

include/aare/NDView.hpp

@@ -71,7 +71,7 @@ template <typename T, int64_t Ndim = 2> class NDView : public ArrayExpr<NDView<T
         return buffer_[element_offset(strides_, index...)];
     }
 
-    size_t size() const { return size_; }
+    ssize_t size() const { return static_cast<ssize_t>(size_); }
     size_t total_bytes() const { return size_ * sizeof(T); }
     std::array<int64_t, Ndim> strides() const noexcept { return strides_; }
 
@@ -102,7 +102,7 @@ template <typename T, int64_t Ndim = 2> class NDView : public ArrayExpr<NDView<T
 
     template<size_t Size>
     NDView& operator=(const std::array<T, Size> &arr) {
-        if(size() != arr.size())
+        if(size() != static_cast<ssize_t>(arr.size()))
             throw std::runtime_error(LOCATION + "Array and NDView size mismatch");
         std::copy(arr.begin(), arr.end(), begin());
         return *this;

include/aare/VarClusterFinder.hpp

@@ -226,7 +226,7 @@ template <typename T> void VarClusterFinder<T>::single_pass(NDView<T, 2> img) {
 
 template <typename T> void VarClusterFinder<T>::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 <typename T> void VarClusterFinder<T>::first_pass() {
 
 template <typename T> void VarClusterFinder<T>::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);

src/Fit.cpp

@@ -18,7 +18,7 @@ double gaus(const double x, const double *par) {
 
 NDArray<double, 1> gaus(NDView<double, 1> x, NDView<double, 1> par) {
     NDArray<double, 1> 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<double, 1> pol1(NDView<double, 1> x, NDView<double, 1> par) {
     NDArray<double, 1> 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<double, 1> x, NDView<double, 1> y, NDView<double, 1> 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<double, 1> x, NDView<double, 1> y, NDView<double, 1> 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);
     }
 }

src/Interpolator.cpp

@@ -68,19 +68,14 @@ std::vector<Photon> Interpolator::interpolate(const ClusterVector<int32_t>& 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,

src/JungfrauDataFile.cpp

@@ -37,8 +37,9 @@ Frame JungfrauDataFile::read_frame(size_t frame_number){
 
 std::vector<Frame> JungfrauDataFile::read_n(size_t n_frames) {
     std::vector<Frame> 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<ssize_t, 2> JungfrauDataFile::shape() const {
+    return {static_cast<ssize_t>(rows()), static_cast<ssize_t>(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<uint16_t>* 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<std::byte *>(image->data()), header);
 }
 
-NDArray<uint16_t> JungfrauDataFile::read_frame(JungfrauDataHeader* header) {
-    Shape<2> shape{rows(), cols()};
-    NDArray<uint16_t> image(shape);
-
-    read_into(reinterpret_cast<std::byte *>(image.data()),
-            header);
-
-    return image;
-}
 
 JungfrauDataHeader JungfrauDataFile::read_header() {
     JungfrauDataHeader header;

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<uint16_t> 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<uint16_t> 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<uint16_t> 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);
+
+
 }

src/NDArray.test.cpp

@@ -183,14 +183,14 @@ TEST_CASE("Size and shape matches") {
     int64_t h = 75;
     std::array<int64_t, 2> shape{w, h};
     NDArray<double> a{shape};
-    REQUIRE(a.size() == static_cast<uint64_t>(w * h));
+    REQUIRE(a.size() == w * h);
     REQUIRE(a.shape() == shape);
 }
 
 TEST_CASE("Initial value matches for all elements") {
     double v = 4.35;
     NDArray<double> 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);
     }
 }

src/algorithm.test.cpp

@@ -6,7 +6,7 @@
 
 TEST_CASE("Find the closed index in a 1D array", "[algorithm]") {
     aare::NDArray<double, 1> 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<int, 1> 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<double, 1> 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<double, 1> 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<double, 1> 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<double, 1> 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