From 23a4e58ba9f5dbf095666b64bc68404f8b97d455 Mon Sep 17 00:00:00 2001
From: Giovanna Lazzari Miotto <giovanna.lazzari.miotto@cern.ch>
Date: Wed, 8 May 2024 15:10:36 +0200
Subject: [PATCH] fix: proc: Keep same muon data ordering on output

Since 2023, the muon processor had been writing all the first muons in
the packet ahead of the all the second muons existing in the BX. Now the
 same incoming order is kept for the output.

Because BxData now holds both muons in the BX, the maximum amount
comported is doubled (eight rather than four).

Fixes issue #63 (wrong muon sorting).
---
 src/muon_orbit_processor.cc | 23 ++++++++++++++---------
 src/orbit_processor.cc      |  3 +++
 src/orbit_processor.h       |  9 +++++++--
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/muon_orbit_processor.cc b/src/muon_orbit_processor.cc
index 46b6ee61..0832cf2d 100644
--- a/src/muon_orbit_processor.cc
+++ b/src/muon_orbit_processor.cc
@@ -19,7 +19,8 @@ int MuonOrbitProcessor::ProcessBlock(MemRegion &readable_block, WriteMemRegion &
   assert(readable_block.CheckBounds(sizeof(SourceDataType)));
   SourceDataType *bl = readable_block.Scan<SourceDataType>();
 
-  BxData<SinkDataType, source_data_length> bx_data_1, bx_data_2;
+  BxData<SinkDataType, 2 * source_data_length> bx_data;
+  uint32_t count_m1 = 0, count_m2 = 0;
 
   for (unsigned int i = 0; i < source_data_length; i++) {
     const auto pt1 = uint32_t{(bl->mu1f[i] >> shifts::pt) & masks::pt};
@@ -28,29 +29,33 @@ int MuonOrbitProcessor::ProcessBlock(MemRegion &readable_block, WriteMemRegion &
     // mu.extra is a copy of bl->bx with a change to the first bit.
     if (((pt1 > 0) || (doZS == 0))) {
       // set bit0 to "0" for first muon
-      bx_data_1.Add({bl->mu1f[i], bl->mu1s[i], bl->bx[i] &= ~0x1});
+      bx_data.Add({bl->mu1f[i], bl->mu1s[i], bl->bx[i] &= ~0x1});
+      count_m1 += 1;
     }
     if (((pt2 > 0) || (doZS == 0))) {
       // set bit0 to "1" for second muon
-      bx_data_2.Add({bl->mu2f[i], bl->mu2s[i], bl->bx[i] |= 0x1});
+      bx_data.Add({bl->mu2f[i], bl->mu2s[i], bl->bx[i] |= 0x1});
+      count_m2 += 1;
     }
   }
 
-  if (bx_data_1.count + bx_data_2.count == 0) {
+  if (bx_data.count == 0) {
     LOG(WARNING) << '#' << nbPackets
                  << ": Detected a bx with zero muons, this should not "
                     "happen. Packet is skipped.";
     return -1;
   }
 
+  LOG(DEBUG) << "Filling data with total size = " << std::to_string(bx_data.GetSize())
+             << ", data points = " << std::to_string(bx_data.count)
+             << ", available space = " << std::to_string(writeable_block.GetAvailable());
+
   // header word of packed muon data contains number of muons in words 3-4 and number of muons in
   // words 5-6, as well as the warning test enable flag.
-  meta.header = uint32_t{(bx_data_1.count << 16) + ((static_cast<uint32_t>(meta.header)) << 8) +
-                         bx_data_2.count};
+  meta.header = uint32_t{(count_m1 << 16) + ((static_cast<uint32_t>(meta.header)) << 8) + count_m2};
 
   writeable_block.Fill(BxMetadata{meta.header, meta.bx, meta.orbit});
-  writeable_block.Fill(bx_data_1.GetBuffer(), bx_data_1.GetSize());
-  writeable_block.Fill(bx_data_2.GetBuffer(), bx_data_2.GetSize());
+  writeable_block.Fill(bx_data.GetBuffer(), bx_data.GetSize());
 
-  return bx_data_1.count + bx_data_2.count;
+  return bx_data.count;
 }
diff --git a/src/orbit_processor.cc b/src/orbit_processor.cc
index 4dda9c57..4cdc8f3b 100644
--- a/src/orbit_processor.cc
+++ b/src/orbit_processor.cc
@@ -140,6 +140,9 @@ OrbitProcessor::FillOrbitMetadata OrbitProcessor::FillOrbit(orbit_trailer *trail
     // fill
     MemRegion readable_block(&rd_ptr, rd_end_ptr);
     WriteMemRegion writeable_block(&wr_ptr, wr_end_ptr);
+
+    LOG(DEBUG) << "Writing to memory region: " << std::to_string(writeable_block.GetAvailable())
+               << " bytes available, " << std::to_string(GetPacketSize()) << " needed.";
     assert(writeable_block.CheckBounds(GetPacketSize()));  // Max size a decoded block can use
 
     BxMetadata meta{orbit_header.second, static_cast<uint32_t>(bx), orbit};
diff --git a/src/orbit_processor.h b/src/orbit_processor.h
index 1686a93f..f95c0225 100644
--- a/src/orbit_processor.h
+++ b/src/orbit_processor.h
@@ -63,7 +63,10 @@ class OrbitProcessor : public Processor {
 
     T *GetBuffer() { return data; }
 
-    void Add(T v) { data[count++] = v; }
+    void Add(T v) {
+      assert(count < N);
+      data[count++] = v;
+    }
 
     const static uint32_t max_elem = N;
     T data[max_elem];
@@ -84,7 +87,7 @@ class OrbitProcessor : public Processor {
    public:
     MemRegion(char **ptr, const char *end) : ptr_(ptr), ptr_end_(end) {}
 
-    bool CheckBounds(size_t size) { return *ptr_ + size < ptr_end_; }
+    bool CheckBounds(size_t size) { return (GetAvailable() - static_cast<int64_t>(size)) >= 0; }
 
     void Increment(size_t size) { (*ptr_) += size; }
 
@@ -97,6 +100,8 @@ class OrbitProcessor : public Processor {
       return view;
     }
 
+    int64_t GetAvailable() { return ptr_end_ - *ptr_; }
+
    protected:
     char **ptr_;
     const char *ptr_end_;
-- 
GitLab