Developer (#102)

CMakeLists.txt

@@ -241,6 +241,7 @@ target_compile_options(
         -Wextra 
         -pedantic 
         -Wshadow 
+        -Wold-style-cast
         -Wnon-virtual-dtor
         -Woverloaded-virtual
         -Wdouble-promotion
@@ -413,7 +414,7 @@ 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 "${CLANG_TIDY_COMMAND} --config-file=.clang-tidy -p build {}"
+    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 "${CLANG_TIDY_COMMAND} --config-file=.clang-tidy -p build {}"
     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
     COMMENT "linting with clang-tidy"
     VERBATIM

include/aare/ClusterFile.hpp

@@ -47,6 +47,7 @@ class ClusterFile {
 
   public:
     ClusterFile(const std::filesystem::path &fname, size_t chunk_size = 1000);
+    ~ClusterFile();
     std::vector<Cluster> read_clusters(size_t n_clusters);
     std::vector<Cluster> read_frame(int32_t &out_fnum);
     std::vector<Cluster>
@@ -59,6 +60,7 @@ class ClusterFile {
                     double *eta3y);
 
     size_t chunk_size() const { return m_chunk_size; }
+    void close();
 
 };
 

include/aare/RawMasterFile.hpp

@@ -61,14 +61,14 @@ class ScanParameters {
 
 
 struct ROI{
-  size_t xmin{};
+  int64_t xmin{};
-  size_t xmax{};
+  int64_t xmax{};
-  size_t ymin{};
+  int64_t ymin{};
-  size_t ymax{};
+  int64_t ymax{};
 
-  size_t height() const { return ymax - ymin; }
+  int64_t height() const { return ymax - ymin; }
-  size_t width() const { return xmax - xmin; }
+  int64_t width() const { return xmax - xmin; }
-}__attribute__((packed));
+};
 
 
 /**
@@ -91,7 +91,7 @@ class RawMasterFile {
     xy m_geometry;
 
     size_t m_max_frames_per_file{};
-    uint32_t m_adc_mask{};
+    // uint32_t m_adc_mask{}; // TODO! implement reading
     FrameDiscardPolicy m_frame_discard_policy{};
     size_t m_frame_padding{};
 

include/aare/RawSubFile.hpp

@@ -16,6 +16,7 @@ namespace aare {
 class RawSubFile {
   protected:
     std::ifstream m_file;
+    DetectorType m_detector_type;
     size_t m_bitdepth;
     std::filesystem::path m_fname;
     size_t m_rows{};
@@ -25,7 +26,7 @@ class RawSubFile {
     uint32_t m_pos_row{};
     uint32_t m_pos_col{};
  
-    DetectorType m_detector_type;
+    
     std::optional<NDArray<ssize_t, 2>> m_pixel_map;
 
   public:

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 (int i = 0; i < original_.size(); ++i) {
+    for (size_t i = 0; i < original_.size(); ++i) {
         if (use_noise_map)
             threshold_ = 5 * noiseMap(i);
         binary_(i) = (original_(i) > threshold_);
@@ -250,17 +250,17 @@ template <typename T> void VarClusterFinder<T>::first_pass() {
 
 template <typename T> void VarClusterFinder<T>::second_pass() {
 
-    for (int64_t i = 0; i != labeled_.size(); ++i) {
+    for (size_t i = 0; i != labeled_.size(); ++i) {
-        auto current_label = labeled_(i);
+        auto cl = labeled_(i);
-        if (current_label != 0) {
+        if (cl != 0) {
-            auto it = child.find(current_label);
+            auto it = child.find(cl);
             while (it != child.end()) {
-                current_label = it->second;
+                cl = it->second;
-                it = child.find(current_label);
+                it = child.find(cl);
                 // do this once before doing the second pass?
                 // all values point to the final one...
             }
-            labeled_(i) = current_label;
+            labeled_(i) = cl;
         }
     }
 }
@@ -271,7 +271,7 @@ template <typename T> void VarClusterFinder<T>::store_clusters() {
     // Do we always have monotonic increasing
     // labels? Then vector?
     // here the translation is label -> Hit
-    std::unordered_map<int, Hit> h_size;
+    std::unordered_map<int, Hit> h_map;
     for (int i = 0; i < shape_[0]; ++i) {
         for (int j = 0; j < shape_[1]; ++j) {
             if (labeled_(i, j) != 0 || false
@@ -280,7 +280,7 @@ template <typename T> void VarClusterFinder<T>::store_clusters() {
                 // (i+1 < shape_[0] and labeled_(i+1, j) != 0) or
                 // (j+1 < shape_[1] and labeled_(i, j+1) != 0)
             ) {
-                Hit &record = h_size[labeled_(i, j)];
+                Hit &record = h_map[labeled_(i, j)];
                 if (record.size < MAX_CLUSTER_SIZE) {
                     record.rows[record.size] = i;
                     record.cols[record.size] = j;
@@ -300,7 +300,7 @@ template <typename T> void VarClusterFinder<T>::store_clusters() {
         }
     }
 
-    for (const auto &h : h_size)
+    for (const auto &h : h_map)
         hits.push_back(h.second);
 }
 

include/aare/defs.hpp

@@ -93,7 +93,7 @@ class DynamicCluster {
         (sizeof(T) == dt.bytes())
             ? 0
             : throw std::invalid_argument("[ERROR] Type size mismatch");
-        return memcpy(m_data + idx * dt.bytes(), &val, (size_t)dt.bytes());
+        return memcpy(m_data + idx * dt.bytes(), &val, dt.bytes());
     }
 
     template <typename T> std::string to_string() const {

python/src/cluster_file.hpp

@@ -37,7 +37,7 @@ void define_cluster_file_io_bindings(py::module &m) {
                  return return_vector(vec);
              })
         .def("__enter__", [](ClusterFile &self) { return &self; })
-        .def("__exit__", [](ClusterFile &self, py::args args) { return; })
+        .def("__exit__", [](ClusterFile &self) {  self.close();})
         .def("__iter__", [](ClusterFile &self) { return &self; })
         .def("__next__", [](ClusterFile &self) {
             auto vec =  new std::vector<Cluster>(self.read_clusters(self.chunk_size()));

python/src/file.hpp

@@ -194,7 +194,7 @@ void define_file_io_bindings(py::module &m) {
             return fmt::format("<ROI: xmin: {} xmax: {} ymin: {} ymax: {}>", self.xmin, self.xmax, self.ymin, self.ymax);
         })
         .def("__iter__", [](const ROI &self) {
-            return py::make_iterator(&self.xmin, &self.ymax+1);
+            return py::make_iterator(&self.xmin, &self.ymax+1); //NOLINT
         });
 
     

python/src/raw_file.hpp

@@ -25,7 +25,6 @@ void define_raw_file_io_bindings(py::module &m) {
         .def(py::init<const std::filesystem::path &>())
         .def("read_frame",
              [](RawFile &self) {
-                 size_t image_size = self.bytes_per_frame();
                  py::array image;
                  std::vector<ssize_t> shape;
                  shape.reserve(2);

src/ClusterFile.cpp

@@ -9,6 +9,17 @@ ClusterFile::ClusterFile(const std::filesystem::path &fname, size_t chunk_size):
     }
 }
 
+ClusterFile::~ClusterFile() {
+    close();
+}
+
+void ClusterFile::close(){
+    if (fp){
+        fclose(fp);
+        fp = nullptr;
+    }  
+}
+
 std::vector<Cluster> ClusterFile::read_clusters(size_t n_clusters) {
     std::vector<Cluster> clusters(n_clusters);
 
@@ -27,7 +38,7 @@ std::vector<Cluster> ClusterFile::read_clusters(size_t n_clusters) {
         } else {
             nn = nph;
         }
-        nph_read += fread((void *)(buf + nph_read), sizeof(Cluster), nn, fp);
+        nph_read += fread(reinterpret_cast<void *>(buf + nph_read), sizeof(Cluster), nn, fp);
         m_num_left = nph - nn; // write back the number of photons left
     }
 
@@ -42,7 +53,7 @@ std::vector<Cluster> ClusterFile::read_clusters(size_t n_clusters) {
                     nn = nph;
 
                 nph_read +=
-                    fread((void *)(buf + nph_read), sizeof(Cluster), nn, fp);
+                    fread(reinterpret_cast<void *>(buf + nph_read), sizeof(Cluster), nn, fp);
                 m_num_left = nph - nn;
             }
             if (nph_read >= n_clusters)
@@ -65,13 +76,13 @@ std::vector<Cluster> ClusterFile::read_frame(int32_t &out_fnum) {
         throw std::runtime_error("Could not read frame number");
     }
 
-    int n_clusters;
+    int32_t n_clusters; // Saved as 32bit integer in the cluster file
     if (fread(&n_clusters, sizeof(n_clusters), 1, fp) != 1) {
         throw std::runtime_error("Could not read number of clusters");
     }
     std::vector<Cluster> clusters(n_clusters);
 
-    if (fread(clusters.data(), sizeof(Cluster), n_clusters, fp) != n_clusters) {
+    if (fread(clusters.data(), sizeof(Cluster), n_clusters, fp) != static_cast<size_t>(n_clusters)) {
         throw std::runtime_error("Could not read clusters");
     }
     return clusters;
@@ -113,7 +124,7 @@ std::vector<Cluster> ClusterFile::read_cluster_with_cut(size_t n_clusters,
         }
         for (size_t iph = 0; iph < nn; iph++) {
             // read photons 1 by 1
-            size_t n_read = fread((void *)(ptr), sizeof(Cluster), 1, fp);
+            size_t n_read = fread(reinterpret_cast<void *>(ptr), sizeof(Cluster), 1, fp);
             if (n_read != 1) {
                 clusters.resize(nph_read);
                 return clusters;
@@ -158,7 +169,7 @@ std::vector<Cluster> ClusterFile::read_cluster_with_cut(size_t n_clusters,
                     for (size_t iph = 0; iph < nph; iph++) {
     //                     // read photons 1 by 1
                         size_t n_read =
-                            fread((void *)(ptr), sizeof(Cluster), 1, fp);
+                            fread(reinterpret_cast<void *>(ptr), sizeof(Cluster), 1, fp);
                         if (n_read != 1) {
                             clusters.resize(nph_read);
                             return clusters;
@@ -269,27 +280,27 @@ int ClusterFile::analyze_data(int32_t *data, int32_t *t2, int32_t *t3, char *qua
         switch (c) {
         case cBottomLeft:
             if (eta2x && (data[3] + data[4]) != 0)
-                *eta2x = (double)(data[4]) / (data[3] + data[4]);
+                *eta2x = static_cast<double>(data[4]) / (data[3] + data[4]);
             if (eta2y && (data[1] + data[4]) != 0)
-                *eta2y = (double)(data[4]) / (data[1] + data[4]);
+                *eta2y = static_cast<double>(data[4]) / (data[1] + data[4]);
             break;
         case cBottomRight:
             if (eta2x && (data[2] + data[5]) != 0)
-                *eta2x = (double)(data[5]) / (data[4] + data[5]);
+                *eta2x = static_cast<double>(data[5]) / (data[4] + data[5]);
             if (eta2y && (data[1] + data[4]) != 0)
-                *eta2y = (double)(data[4]) / (data[1] + data[4]);
+                *eta2y = static_cast<double>(data[4]) / (data[1] + data[4]);
             break;
         case cTopLeft:
             if (eta2x && (data[7] + data[4]) != 0)
-                *eta2x = (double)(data[4]) / (data[3] + data[4]);
+                *eta2x = static_cast<double>(data[4]) / (data[3] + data[4]);
             if (eta2y && (data[7] + data[4]) != 0)
-                *eta2y = (double)(data[7]) / (data[7] + data[4]);
+                *eta2y = static_cast<double>(data[7]) / (data[7] + data[4]);
             break;
         case cTopRight:
             if (eta2x && t2max != 0)
-                *eta2x = (double)(data[5]) / (data[5] + data[4]);
+                *eta2x = static_cast<double>(data[5]) / (data[5] + data[4]);
             if (eta2y && t2max != 0)
-                *eta2y = (double)(data[7]) / (data[7] + data[4]);
+                *eta2y = static_cast<double>(data[7]) / (data[7] + data[4]);
             break;
         default:;
         }
@@ -297,10 +308,10 @@ int ClusterFile::analyze_data(int32_t *data, int32_t *t2, int32_t *t3, char *qua
 
     if (eta3x || eta3y) {
         if (eta3x && (data[3] + data[4] + data[5]) != 0)
-            *eta3x = (double)(-data[3] + data[3 + 2]) /
+            *eta3x = static_cast<double>(-data[3] + data[3 + 2]) /
                      (data[3] + data[4] + data[5]);
         if (eta3y && (data[1] + data[4] + data[7]) != 0)
-            *eta3y = (double)(-data[1] + data[2 * 3 + 1]) /
+            *eta3y = static_cast<double>(-data[1] + data[2 * 3 + 1]) /
                      (data[1] + data[4] + data[7]);
     }
 

src/RawFile.cpp

@@ -99,10 +99,7 @@ void RawFile::open_subfiles() {
         for (size_t i = 0; i != n_subfiles; ++i) {
             auto v = std::vector<RawSubFile *>(n_subfile_parts);
             for (size_t j = 0; j != n_subfile_parts; ++j) {
-                fmt::print("{} pos: {},{}\n", j,positions[j].row, positions[j].col);
-                
                 auto pos = m_module_pixel_0[j];
-                fmt::print("{} pos: {},{}\n", j,pos.y, pos.x);
                 v[j] = new RawSubFile(m_master.data_fname(j, i),
                                       m_master.detector_type(), pos.height,
                                       pos.width, m_master.bitdepth(),
@@ -349,7 +346,7 @@ void RawFile::get_frame_into(size_t frame_index, std::byte *frame_buffer, Detect
             if(header)
                 ++header;
 
-            for (size_t cur_row = 0; cur_row < (pos.height);
+            for (size_t cur_row = 0; cur_row < static_cast<size_t>(pos.height);
                  cur_row++) {
 
                 auto irow = (pos.y + cur_row);

src/RawSubFile.cpp

@@ -9,8 +9,7 @@ namespace aare {
 RawSubFile::RawSubFile(const std::filesystem::path &fname,
                        DetectorType detector, size_t rows, size_t cols,
                        size_t bitdepth, uint32_t pos_row, uint32_t pos_col)
-    : m_bitdepth(bitdepth), m_fname(fname), m_rows(rows), m_cols(cols),
+    : m_detector_type(detector), m_bitdepth(bitdepth), m_fname(fname), m_rows(rows), m_cols(cols),
-      m_detector_type(detector),
       m_bytes_per_frame((m_bitdepth / 8) * m_rows * m_cols), m_pos_row(pos_row), m_pos_col(pos_col) {
     if (m_detector_type == DetectorType::Moench03_old) {
         m_pixel_map = GenerateMoench03PixelMap();