Skip to content

Commit 2035d65

Browse files
njacazionjacazio
authored
TOF: Improve Counter class (#627)
- Add unit test of counter - Add 1:1 between histogram and labels
1 parent 51200a3 commit 2035d65

3 files changed

Lines changed: 195 additions & 60 deletions

File tree

‎Modules/TOF/include/Base/Counter.h‎

Lines changed: 117 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -69,24 +69,32 @@ class Counter
6969

7070
/// Function to get how many counts where observed
7171
/// @return Returns the number of counts observed for a particular index
72-
uint32_t HowMany(const unsigned int& pos) const { return counter[pos]; }
72+
uint32_t HowMany(const unsigned int& index) const { return counter[index]; }
7373

74-
/// Function to make a histogram out of the counters
75-
/// @param h histogram to shape in order to have room for the counter size
76-
void MakeHistogram(TH1* h) const;
74+
/// Function to check if the counter has a label at position index
75+
/// @param index Index in the counter array to be checked
76+
/// @return Returns if the counter has label corresponding to a particular index. Returns false if the label is not there or is empty.
77+
constexpr bool HasLabel(const unsigned int& index) const;
78+
79+
/// Function to make a histogram out of the counters. If a counter has labels defined these are used as axis labels, if not this will not be done.
80+
/// @param histogram histogram to shape in order to have room for the counter size
81+
/// @returns Returns 0 if everything went OK
82+
int MakeHistogram(TH1* histogram) const;
7783

7884
/// Function to fill a histogram with the counters
79-
/// @param h The histogram to fill
85+
/// @param histogram The histogram to fill
8086
/// @param biny Y offset to fill to histogram, useful for TH2 and TH3
8187
/// @param binz Z offset to fill to histogram, useful for TH3
82-
void FillHistogram(TH1* h, const unsigned int& biny = 0, const unsigned int& binz = 0) const;
88+
/// @returns Returns 0 if everything went OK
89+
int FillHistogram(TH1* histogram, const unsigned int& biny = 0, const unsigned int& binz = 0) const;
8390

8491
/// Getter for the size
8592
/// @return Returns the size of the counter
8693
unsigned int Size() const { return size; }
8794

8895
private:
89-
// static_assert((sizeof(labels) / sizeof(const char*)) == size, "size of the counter and the one of the labels must coincide");
96+
static_assert(size > 0, "size of the counter cannot be 0!");
97+
// static_assert(((labels == nullptr) || (sizeof(labels) / sizeof(const char*)) == size), "size of the counter and the one of the labels must coincide");
9098
/// Containers to fill
9199
std::array<uint32_t, size> counter = { 0 };
92100
uint32_t mTotal = 0;
@@ -96,18 +104,15 @@ class Counter
96104
// Implementation //
97105
////////////////////
98106

99-
// #define ENABLE_COUNTER_DEBUG_MODE // Flag used to enable more printing and more debug
100-
// #define ENABLE_PRINT_HISTOGRAMS_MODE // Flag used to enable more printing and more debug
107+
// #define ENABLE_BIN_SHIFT // Flag used to enable different binning in counter and histograms
101108

102109
template <const unsigned int size, const char* labels[size]>
103110
void Counter<size, labels>::Add(const unsigned int& index, const uint32_t& weight)
104111
{
105112
if (index > size) {
106113
LOG(FATAL) << "Incrementing counter too far! " << index << "/" << size;
107114
}
108-
#ifdef ENABLE_COUNTER_DEBUG_MODE
109-
LOG(INFO) << "Incrementing " << index << "/" << size << " of " << weight << " to " << counter[index];
110-
#endif
115+
LOG(DEBUG) << "Incrementing " << index << "/" << size << " of " << weight << " to " << counter[index];
111116
counter[index] += weight;
112117
}
113118

@@ -120,14 +125,23 @@ void Counter<size, labels>::Reset()
120125
}
121126
}
122127

128+
template <const unsigned int size, const char* labels[size]>
129+
constexpr bool Counter<size, labels>::HasLabel(const unsigned int& index) const
130+
{
131+
if constexpr (labels != nullptr) {
132+
return (labels[index] && labels[index][0]);
133+
}
134+
return false;
135+
}
136+
123137
template <const unsigned int size, const char* labels[size]>
124138
void Counter<size, labels>::Print()
125139
{
126140
for (unsigned int i = 0; i < size; i++) {
127141
if (labels != nullptr) {
128-
LOG(DEBUG) << "Bin " << i << "/" << size - 1 << " " << labels[i] << " = " << HowMany(i);
142+
LOG(INFO) << "Bin " << i << "/" << size - 1 << " '" << labels[i] << "' = " << HowMany(i);
129143
} else {
130-
LOG(DEBUG) << "Bin " << i << "/" << size - 1 << " = " << HowMany(i);
144+
LOG(INFO) << "Bin " << i << "/" << size - 1 << " = " << HowMany(i);
131145
}
132146
}
133147
}
@@ -159,92 +173,139 @@ uint32_t Counter<size, labels>::TotalAndReset()
159173
}
160174

161175
template <const unsigned int size, const char* labels[size]>
162-
void Counter<size, labels>::MakeHistogram(TH1* h) const
176+
int Counter<size, labels>::MakeHistogram(TH1* histogram) const
163177
{
164-
LOG(DEBUG) << "Making Histogram " << h->GetName() << " to accomodate counter of size " << size;
165-
TAxis* axis = h->GetXaxis();
178+
LOG(DEBUG) << "Making Histogram " << histogram->GetName() << " to accomodate counter of size " << size;
179+
TAxis* axis = histogram->GetXaxis();
166180
if (static_cast<unsigned int>(axis->GetNbins()) < size) {
167181
LOG(FATAL) << "The histogram size (" << axis->GetNbins() << ") is not large enough to accomodate the counter size (" << size << ")";
182+
return 1;
183+
}
184+
histogram->Reset();
185+
186+
#ifndef ENABLE_BIN_SHIFT
187+
LOG(DEBUG) << "Asked to produce a histogram with size " << size << " out of the size " << size << " due to " << size - size << " empty labels";
188+
axis->Set(size, 0, size);
189+
for (unsigned int i = 0; i < size; i++) {
190+
if (!HasLabel(i)) { // If label at position i is empty
191+
continue;
192+
}
193+
LOG(DEBUG) << "Setting bin " << i + 1 << "/" << size << " to contain counter for '" << labels[i] << "' (index " << i << "/" << size - 1 << ")";
194+
axis->SetBinLabel(i + 1, labels[i]);
168195
}
169-
h->Reset();
196+
#else
170197
unsigned int histo_size = size;
171198
if (labels != nullptr) { // Only if labels are defined
172199
for (unsigned int i = 0; i < size; i++) {
173-
if (labels[i] && !labels[i][0]) { // If label at position i is empty
200+
if (!HasLabel(i)) { // If label at position i is empty or does not exist
174201
LOG(DEBUG) << "Skipping label '" << labels[i] << "' at position " << i << "/" << size - 1;
175202
histo_size--;
176203
}
177204
}
178205
}
179206
if (histo_size == 0) {
180207
LOG(FATAL) << "Asked to produce a histogram with size " << histo_size << ", check counter bin labels";
208+
return 1;
181209
} else {
182-
LOG(DEBUG) << "Asked to produce a histogram with size " << histo_size << " out of the size " << size << " due to empty labels";
210+
LOG(DEBUG) << "Asked to produce a histogram with size " << histo_size << " out of the size " << size << " due to " << size - histo_size << " empty labels";
183211
}
184212

185213
axis->Set(histo_size, 0, histo_size);
186214
if (labels != nullptr) { // Only if labels are defined
187215
unsigned int binx = 1;
188216
for (unsigned int i = 0; i < size; i++) {
189-
if (labels[i] && !labels[i][0]) { // If label at position i is empty
217+
if (!HasLabel(i)) { // If label at position i is empty
190218
continue;
191219
}
192-
LOG(DEBUG) << "Setting bin " << binx << "/" << histo_size << " to contain counter for '" << labels[i] << "' (index " << i << "/" << size - 1 << ")";
193220
if (histo_size < binx) {
194221
LOG(FATAL) << "Making bin outside of histogram limits!";
222+
return 1;
195223
}
224+
LOG(DEBUG) << "Setting bin " << binx << "/" << histo_size << " to contain counter for '" << labels[i] << "' (index " << i << "/" << size - 1 << ")";
196225
axis->SetBinLabel(binx++, labels[i]);
197226
}
198227
}
199-
h->Reset();
200-
#ifdef ENABLE_PRINT_HISTOGRAMS_MODE
201-
h->Print("All");
202228
#endif
229+
histogram->Reset();
230+
return 0;
203231
}
204232

205233
template <const unsigned int size, const char* labels[size]>
206-
void Counter<size, labels>::FillHistogram(TH1* h, const unsigned int& biny, const unsigned int& binz) const
234+
int Counter<size, labels>::FillHistogram(TH1* histogram, const unsigned int& biny, const unsigned int& binz) const
207235
{
208-
LOG(DEBUG) << "Filling Histogram " << h->GetName() << " with counter contents";
209-
unsigned int binx = 1;
210-
const unsigned int nbinsx = h->GetNbinsX();
211-
for (unsigned int i = 0; i < size; i++) {
212-
if (labels != nullptr && labels[i] && !labels[i][0]) { // Labels are defined and label is empty
213-
if (counter[i] > 0) {
214-
LOG(FATAL) << "Counter at position " << i << " was non empty (" << counter[i] << ") but was discarded because of empty labels";
236+
auto fillIt = [&](const unsigned int& bin, const unsigned int& index) {
237+
if (counter[index] > 0) {
238+
if (biny > 0) {
239+
if (binz > 0) {
240+
histogram->SetBinContent(bin, biny, binz, counter[index]);
241+
} else {
242+
histogram->SetBinContent(bin, biny, counter[index]);
243+
}
244+
} else {
245+
histogram->SetBinContent(bin, counter[index]);
215246
}
216-
continue;
217247
}
218-
#ifdef ENABLE_COUNTER_DEBUG_MODE
219-
LOG(INFO) << "Filling bin " << binx << " of position " << i << " of label " << labels[i] << " with " << counter[i];
220-
#endif
221-
if (binx > nbinsx) {
222-
LOG(FATAL) << "Filling histogram " << h->GetName() << " at position " << binx << " i.e. past its size (" << nbinsx << ")!";
248+
};
249+
250+
LOG(DEBUG) << "Filling Histogram " << histogram->GetName() << " with counter contents";
251+
#ifndef ENABLE_BIN_SHIFT
252+
if (size != (histogram->GetNbinsX())) {
253+
LOG(FATAL) << "Counter of size " << size << " does not fit in histogram " << histogram->GetName() << " with size " << histogram->GetNbinsX() - 1;
254+
return 1;
255+
}
256+
for (unsigned int i = 0; i < size; i++) {
257+
LOG(DEBUG) << "Filling bin " << i + 1 << " with counter at position " << i;
258+
if (HasLabel(i) && strcmp(labels[i], histogram->GetXaxis()->GetBinLabel(i + 1)) != 0) { // If it has a label check its consistency!
259+
LOG(FATAL) << "Bin " << i + 1 << " does not have the expected label '" << histogram->GetXaxis()->GetBinLabel(i + 1) << "' vs '" << labels[i] << "'";
260+
return 1;
223261
}
224-
const char* bin_label = h->GetXaxis()->GetBinLabel(binx);
225-
if (labels != nullptr) {
226-
if (!labels[i]) {
227-
LOG(DEBUG) << "Label at position " << i << " does not exist for axis label '" << bin_label << "'";
262+
fillIt(i + 1, i);
263+
}
264+
#else
265+
const unsigned int nbinsx = histogram->GetNbinsX();
266+
if constexpr (labels == nullptr) { // Fill without labels
267+
if (nbinsx < size) {
268+
LOG(FATAL) << "Counter size " << size << " is too large to fit in histogram " << histogram->GetName() << " with size " << nbinsx;
269+
return 1;
270+
}
271+
for (unsigned int i = 0; i < size; i++) {
272+
LOG(DEBUG) << "Filling bin " << i + 1 << " with position " << i << " with " << counter[i];
273+
fillIt(i + 1, i);
274+
}
275+
} else { // Fill with labels
276+
unsigned int binx = 1;
277+
for (unsigned int i = 0; i < size; i++) {
278+
if (!HasLabel(i)) { // Labels are defined and label is empty
279+
if (counter[i] > 0) {
280+
LOG(FATAL) << "Counter at position " << i << " was non empty (" << counter[i] << ") but was discarded because of empty labels";
281+
return 1;
282+
}
283+
continue;
284+
}
285+
LOG(DEBUG) << "Filling bin " << binx << " with position " << i << " of label " << labels[i] << " with " << counter[i];
286+
if (binx > nbinsx) {
287+
LOG(FATAL) << "Filling histogram " << histogram->GetName() << " at position " << binx << " i.e. past its size (" << nbinsx << ")!";
288+
return 1;
289+
}
290+
const char* bin_label = histogram->GetXaxis()->GetBinLabel(binx);
291+
if (!HasLabel(i) && counter[i] > 0) {
292+
LOG(FATAL) << "Label at position " << i << " does not exist for axis label '" << bin_label << "' but counter is *non* empty!";
293+
return 1;
228294
} else if (strcmp(labels[i], bin_label) != 0) {
229295
LOG(FATAL) << "Bin " << binx << " does not have the expected label '" << bin_label << "' vs '" << labels[i] << "'";
296+
return 1;
230297
}
298+
fillIt(binx, i);
299+
binx++;
231300
}
232-
if (counter[i] > 0) {
233-
if (biny > 0) {
234-
if (binz > 0) {
235-
h->SetBinContent(binx, biny, binz, counter[i]);
236-
} else {
237-
h->SetBinContent(binx, biny, counter[i]);
238-
}
239-
} else {
240-
h->SetBinContent(binx, counter[i]);
241-
}
301+
if (binx != nbinsx + 1) {
302+
LOG(FATAL) << "Did not fully fill histogram " << histogram->GetName() << ", filled " << binx << " out of " << nbinsx;
303+
return 1;
242304
}
243-
binx++;
244305
}
245-
#ifdef ENABLE_PRINT_HISTOGRAMS_MODE
246-
h->Print("All");
247306
#endif
307+
308+
return 0;
248309
}
249310

250311
} // namespace o2::quality_control_modules::tof

‎Modules/TOF/src/TaskRaw.cxx‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -439,12 +439,14 @@ void TaskRaw::endOfCycle()
439439
diagnosticHisto = mHistoTRM[slot - 2].get();
440440
}
441441
if (diagnosticHisto) {
442-
for (unsigned int crate = 0; crate < RawDataDecoder::ncrates; crate++) { // Loop over crates
443-
for (unsigned int word = 0; word < RawDataDecoder::nwords; word++) { // Loop over words
444-
mHistoCrate[crate]->SetBinContent(word + 1, slot + 2,
442+
for (int crate = 0; crate < diagnosticHisto->GetNbinsX(); crate++) { // Loop over crates
443+
for (int word = 0; word < diagnosticHisto->GetNbinsY(); word++) { // Loop over words
444+
mHistoCrate[crate]->SetBinContent(word + 1, slot + 2, // Shift position 1 to make room for the RDH
445445
diagnosticHisto->GetBinContent(word + 1, crate + 1));
446446
}
447447
}
448+
} else {
449+
LOG(WARNING) << "Did not find diagnostic histogram for slot " << slot << " for reshuffling";
448450
}
449451
}
450452
for (unsigned int crate = 0; crate < RawDataDecoder::ncrates; crate++) { // Loop over crates for how many RDH read

‎Modules/TOF/test/testTOF.cxx‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
///
1515

1616
#include "QualityControl/TaskFactory.h"
17+
#include "Base/Counter.h"
18+
#include "DataFormatsTOF/CompressedDataFormat.h"
19+
#include "TH1F.h"
1720

1821
#define BOOST_TEST_MODULE Publisher test
1922
#define BOOST_TEST_MAIN
@@ -26,4 +29,73 @@ namespace o2::quality_control_modules::tof
2629

2730
BOOST_AUTO_TEST_CASE(instantiate_task) { BOOST_CHECK(true); }
2831

29-
} // namespace o2::quality_control_modules::tof
32+
BOOST_AUTO_TEST_CASE(check_tof_counter)
33+
{
34+
BOOST_TEST_CHECKPOINT("Starting");
35+
36+
TH1F* hFull = new TH1F("hFull", "hFull;DRM Word;Crate;Words", 32, 0, 32);
37+
Counter<32, o2::tof::diagnostic::DRMDiagnosticName> counterFull;
38+
BOOST_CHECK_MESSAGE(counterFull.MakeHistogram(hFull) == 0, "Did not correctly make the histogram for Full labels");
39+
int nFull = 0;
40+
for (int i = 0; i < 32; i++) {
41+
if (o2::tof::diagnostic::DRMDiagnosticName[i] && o2::tof::diagnostic::DRMDiagnosticName[i][0]) {
42+
nFull++;
43+
}
44+
}
45+
46+
TH1F* hEmpty = new TH1F("hEmpty", "hEmpty;DRM Word;Crate;Words", 32, 0, 32);
47+
Counter<32, nullptr> counterEmpty;
48+
BOOST_CHECK_MESSAGE(counterEmpty.MakeHistogram(hEmpty) == 0, "Did not correctly make the histogram for Empty labels");
49+
BOOST_CHECK_MESSAGE((hFull->GetNbinsX() == 32) || (hFull->GetNbinsX() + hEmpty->GetNbinsX()) == (32 + nFull),
50+
"Sum of histogram size does not match the number of possible words " << hFull->GetNbinsX() + hEmpty->GetNbinsX() << " vs " << (32 + nFull));
51+
52+
const unsigned int n = 1000;
53+
for (unsigned int i = 0; i < n; i++) {
54+
for (unsigned int j = 0; j < 32; j++) {
55+
if (o2::tof::diagnostic::DRMDiagnosticName[j] && o2::tof::diagnostic::DRMDiagnosticName[j][0]) {
56+
counterFull.Count(j);
57+
} else {
58+
counterEmpty.Count(j);
59+
}
60+
}
61+
}
62+
63+
LOG(INFO) << "Printing counter of full labels";
64+
counterFull.Print();
65+
for (unsigned int j = 0; j < 32; j++) {
66+
unsigned int expected = 0;
67+
if (o2::tof::diagnostic::DRMDiagnosticName[j] && o2::tof::diagnostic::DRMDiagnosticName[j][0]) {
68+
expected = n;
69+
}
70+
BOOST_CHECK_MESSAGE(counterFull.HowMany(j) == expected,
71+
"Issue with the counts in " << o2::tof::diagnostic::DRMDiagnosticName[j] << ": " << counterFull.HowMany(j) << " should be " << expected);
72+
}
73+
BOOST_CHECK_MESSAGE(counterFull.FillHistogram(hFull) == 0, "Did not correctly fill the histogram for Full labels");
74+
BOOST_CHECK_MESSAGE(hFull->Integral() == n * nFull,
75+
"Issue with the counts in histogram of full counter, expected " << n * nFull << " entries and got " << hFull->Integral());
76+
for (int j = 0; j < hFull->GetNbinsX(); j++) {
77+
if (hFull->GetXaxis()->GetBinLabel(j + 1)[0]) {
78+
BOOST_CHECK_MESSAGE(hFull->GetBinContent(j + 1) == n,
79+
"Issue with the counts in bin " << j + 1 << " they must be " << n << " and instead are: " << hFull->GetBinContent(j + 1));
80+
}
81+
LOG(INFO) << "in: " << j + 1 << "/" << hFull->GetNbinsX() + 1 << " (bin '" << hFull->GetXaxis()->GetBinLabel(j + 1) << "') there are " << hFull->GetBinContent(j + 1) << " counts";
82+
}
83+
84+
LOG(INFO) << "Printing counter of empty labels";
85+
counterEmpty.Print();
86+
for (unsigned int j = 0; j < 32; j++) {
87+
unsigned int expected = n;
88+
if (o2::tof::diagnostic::DRMDiagnosticName[j] && o2::tof::diagnostic::DRMDiagnosticName[j][0]) {
89+
expected = 0;
90+
}
91+
BOOST_CHECK_MESSAGE(counterEmpty.HowMany(j) == expected,
92+
"Issue with the counts in " << o2::tof::diagnostic::DRMDiagnosticName[j] << ": " << counterEmpty.HowMany(j) << " should be " << expected);
93+
}
94+
BOOST_CHECK_MESSAGE(counterEmpty.FillHistogram(hEmpty) == 0, "Did not correctly fill the histogram for Empty labels");
95+
BOOST_CHECK_MESSAGE(hEmpty->Integral() == n * (32 - nFull),
96+
"Issue with the counts in histogram of empty counter, expected " << n * (32 - nFull) << " entries and got " << hEmpty->Integral());
97+
98+
BOOST_TEST_CHECKPOINT("Ending");
99+
BOOST_CHECK(true);
100+
}
101+
} // namespace o2::quality_control_modules::tof

0 commit comments

Comments
 (0)