Skip to content

Commit b1e620b

Browse files
ariedel-cernariedel-cern
committed
Feat: add better error reporting for selection containers
1 parent 31acdb7 commit b1e620b

2 files changed

Lines changed: 47 additions & 21 deletions

File tree

‎PWGCF/Femto/Core/baseSelection.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ class BaseSelection
7878
{
7979
// check index
8080
if (static_cast<std::size_t>(observableIndex) >= NumObservables) {
81-
LOG(fatal) << "Observable is not valid. Observable (index) has to be smaller than " << NumObservables;
81+
LOG(fatal) << "Selection '" << selectionName << "': observable is not valid. Observable (index) has to be smaller than " << NumObservables;
8282
}
8383
// init selection container for selection at given index
8484
mSelectionContainers.at(observableIndex) = selectioncontainer::SelectionContainer<T, BitmaskType>(selectionName,
@@ -112,7 +112,7 @@ class BaseSelection
112112
bool isOptionalCut)
113113
{
114114
if (static_cast<std::size_t>(observableIndex) >= NumObservables) {
115-
LOG(fatal) << "Observable is not valid. Observable (index) has to be smaller than " << NumObservables;
115+
LOG(fatal) << "Selection '" << selectionName << "': observable is not valid. Observable (index) has to be smaller than " << NumObservables;
116116
}
117117
mSelectionContainers.at(observableIndex) = selectioncontainer::SelectionContainer<T, BitmaskType>(selectionName,
118118
lowerLimit,
@@ -146,7 +146,7 @@ class BaseSelection
146146
{
147147
// check index
148148
if (static_cast<std::size_t>(observableIndex) >= NumObservables) {
149-
LOG(fatal) << "Observable is not valid. Observable (index) has to be smaller than " << NumObservables;
149+
LOG(fatal) << "Selection '" << selectionName << "': observable is not valid. Observable (index) has to be smaller than " << NumObservables;
150150
}
151151
// init selection container for selection at given index
152152
mSelectionContainers.at(observableIndex) = selectioncontainer::SelectionContainer<T, BitmaskType>(selectionName,
@@ -191,7 +191,7 @@ class BaseSelection
191191
mSelectionContainers.at(observableIndex) = selectioncontainer::SelectionContainer<T, BitmaskType>(selectionName, std::vector<T>{1}, limits::LimitType::kEqual, false, false, false);
192192
break;
193193
default:
194-
LOG(fatal) << "Invalid switch for boolean selection";
194+
LOG(fatal) << "Selection '" << selectionName << "': invalid switch for boolean selection";
195195
}
196196
addSelection(observableIndex);
197197
}
@@ -577,7 +577,7 @@ class BaseSelection
577577
}
578578

579579
if (mNSelectionBits > sizeof(BitmaskType) * CHAR_BIT) {
580-
LOG(fatal) << "Too many selections. At most " << sizeof(BitmaskType) * CHAR_BIT << " number of bits are supported";
580+
LOG(fatal) << "Selection '" << mSelectionContainers.at(observableIndex).getSelectionName() << "': too many selections. At most " << sizeof(BitmaskType) * CHAR_BIT << " number of bits are supported";
581581
}
582582
}
583583

‎PWGCF/Femto/Core/selectionContainer.h‎

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <climits>
2727
#include <cmath>
2828
#include <cstddef>
29+
#include <exception>
2930
#include <limits>
3031
#include <sstream>
3132
#include <string>
@@ -122,10 +123,10 @@ class SelectionContainer
122123
mIsOptionalCut(isOptionalCut)
123124
{
124125
if (mSelectionValues.size() > sizeof(BitmaskType) * CHAR_BIT) {
125-
LOG(fatal) << "Too many selections for single a observable. Limit is " << sizeof(BitmaskType) * CHAR_BIT;
126+
LOG(fatal) << "Selection '" << mSelectionName << "': too many selections for a single observable. Limit is " << sizeof(BitmaskType) * CHAR_BIT;
126127
}
127128
if (isMinimalCut && isOptionalCut) {
128-
LOG(fatal) << "A selection cannot be both minimal and optional";
129+
LOG(fatal) << "Selection '" << mSelectionName << "': a selection cannot be both minimal and optional";
129130
}
130131
// values for selection are not necessarily ordered correctly
131132
sortSelections();
@@ -157,13 +158,13 @@ class SelectionContainer
157158
mIsOptionalCut(isOptionalCut)
158159
{
159160
if (mLimitType != limits::kInsideRange && mLimitType != limits::kOutsideRange) {
160-
LOG(fatal) << "Range-string selections only support kInsideRange or kOutsideRange";
161+
LOG(fatal) << "Selection '" << mSelectionName << "': range-string selections only support kInsideRange or kOutsideRange";
161162
}
162163
if (rangeStrings.size() > sizeof(BitmaskType) * CHAR_BIT) {
163-
LOG(fatal) << "Too many selections for a single observable. Limit is " << sizeof(BitmaskType) * CHAR_BIT;
164+
LOG(fatal) << "Selection '" << mSelectionName << "': too many selections for a single observable. Limit is " << sizeof(BitmaskType) * CHAR_BIT;
164165
}
165166
if (isMinimalCut && isOptionalCut) {
166-
LOG(fatal) << "A selection cannot be both minimal and optional";
167+
LOG(fatal) << "Selection '" << mSelectionName << "': a selection cannot be both minimal and optional";
167168
}
168169
for (auto const& rangeStr : rangeStrings) {
169170
T lower, upper;
@@ -196,7 +197,7 @@ class SelectionContainer
196197
: (mSelectionRanges[i].first <= mSelectionRanges[i + 1].first &&
197198
mSelectionRanges[i].second >= mSelectionRanges[i + 1].second);
198199
if (!nested) {
199-
LOG(fatal) << "Ranges for minimal cut " << selectionName
200+
LOG(fatal) << "Selection '" << selectionName << "': ranges for minimal cut"
200201
<< " are not nested. Range [" << mSelectionRanges[i].first << ";" << mSelectionRanges[i].second
201202
<< "] is not " << (mLimitType == limits::kOutsideRange ? "contained in" : "containing")
202203
<< " [" << mSelectionRanges[i + 1].first << ";" << mSelectionRanges[i + 1].second << "]";
@@ -229,10 +230,10 @@ class SelectionContainer
229230
mIsOptionalCut(isOptionalCut)
230231
{
231232
if (functions.size() > sizeof(BitmaskType) * CHAR_BIT) {
232-
LOG(fatal) << "Too many selections for single a observable. Limit is " << sizeof(BitmaskType) * CHAR_BIT;
233+
LOG(fatal) << "Selection '" << mSelectionName << "': too many selections for a single observable. Limit is " << sizeof(BitmaskType) * CHAR_BIT;
233234
}
234235
if (isMinimalCut && isOptionalCut) {
235-
LOG(fatal) << "A selection cannot be both minimal and optional";
236+
LOG(fatal) << "Selection '" << mSelectionName << "': a selection cannot be both minimal and optional";
236237
}
237238
for (std::size_t i = 0; i < functions.size(); i++) {
238239
mSelectionFunctions.emplace_back((mSelectionName + std::to_string(i)).c_str(), functions.at(i).c_str(), lowerLimit, upperLimit);
@@ -255,7 +256,7 @@ class SelectionContainer
255256
// note: threshold values may be reordered by sortSelections() or sortFunctions(),
256257
// so comments must be provided in the already-sorted order
257258
if (comments.size() != getNSelections()) {
258-
LOG(fatal) << "Number of comments and number of selections are inconsistent";
259+
LOG(fatal) << "Selection '" << mSelectionName << "': number of comments and number of selections are inconsistent";
259260
}
260261
mComments = comments;
261262
}
@@ -382,7 +383,7 @@ class SelectionContainer
382383
void evaluate(std::vector<T> const& values)
383384
{
384385
if (values.size() != mSelectionValues.size()) {
385-
LOG(fatal) << "Wrong number of values have been passed";
386+
LOG(fatal) << "Selection '" << mSelectionName << "': wrong number of values have been passed";
386387
}
387388
// reset the bitmask before evaluating
388389
mBitmask.reset();
@@ -462,7 +463,7 @@ class SelectionContainer
462463
T getLoosestSelection() const
463464
{
464465
if (mSelectionValues.empty()) {
465-
LOG(fatal) << "No selections configured";
466+
LOG(fatal) << "Selection '" << mSelectionName << "': no selections configured";
466467
}
467468
return mSelectionValues.at(0);
468469
}
@@ -605,7 +606,7 @@ class SelectionContainer
605606
[[nodiscard]] int getBitPosition(int selectionIndex) const
606607
{
607608
if (selectionIndex == 0 && mSkipMostPermissiveBit) {
608-
LOG(fatal) << "Trying to accessed the bit position of a skipped selection. Breaking...";
609+
LOG(fatal) << "Selection '" << mSelectionName << "': trying to access the bit position of a skipped selection. Breaking...";
609610
return -1;
610611
}
611612
if (mSkipMostPermissiveBit) {
@@ -706,7 +707,10 @@ class SelectionContainer
706707
{
707708
auto pos = rangeStr.find(';');
708709
if (pos == std::string::npos) {
709-
LOG(fatal) << "Range string '" << rangeStr << "' is missing ';' separator. Expected format: 'lower;upper'";
710+
LOG(fatal) << "Selection '" << mSelectionName << "': range string '" << rangeStr << "' is missing ';' separator. Expected format: 'lower;upper'";
711+
}
712+
if (rangeStr.find(';', pos + 1) != std::string::npos) {
713+
LOG(fatal) << "Selection '" << mSelectionName << "': range string '" << rangeStr << "' has more than one ';' separator. Expected format: 'lower;upper'";
710714
}
711715
std::string lowerStr = rangeStr.substr(0, pos);
712716
std::string upperStr = rangeStr.substr(pos + 1);
@@ -723,14 +727,36 @@ class SelectionContainer
723727
return;
724728
}
725729

726-
lower = lowerStr.empty() ? -std::numeric_limits<T>::max() : static_cast<T>(std::stod(lowerStr));
727-
upper = upperStr.empty() ? std::numeric_limits<T>::max() : static_cast<T>(std::stod(upperStr));
730+
lower = lowerStr.empty() ? -std::numeric_limits<T>::max() : parseBound(rangeStr, lowerStr);
731+
upper = upperStr.empty() ? std::numeric_limits<T>::max() : parseBound(rangeStr, upperStr);
728732

729733
if (lower >= upper) {
730-
LOG(fatal) << "Range string '" << rangeStr << "' has lower bound >= upper bound";
734+
LOG(fatal) << "Selection '" << mSelectionName << "': range string '" << rangeStr << "' has lower bound >= upper bound";
731735
}
732736
}
733737

738+
/// \brief Parse a single range bound and make sure the whole string was consumed, so that
739+
/// malformed input such as "3;5" (extra ';') or "-3,5" (comma used as decimal
740+
/// separator) is rejected instead of silently truncated by std::stod (which happily
741+
/// parses just the numeric prefix "3" or "-3" and ignores the rest).
742+
/// \param rangeStr Full "lower;upper" string, only used for the error message.
743+
/// \param boundStr The individual bound substring (lower or upper) to parse.
744+
T parseBound(std::string const& rangeStr, std::string const& boundStr) const
745+
{
746+
std::size_t idx = 0;
747+
double value = 0.;
748+
try {
749+
value = std::stod(boundStr, &idx);
750+
} catch (std::exception const& e) {
751+
LOG(fatal) << "Selection '" << mSelectionName << "': range string '" << rangeStr << "' contains bound '" << boundStr << "' that is not a valid number (" << e.what() << ")";
752+
}
753+
if (idx != boundStr.size()) {
754+
LOG(fatal) << "Selection '" << mSelectionName << "': range string '" << rangeStr << "' contains bound '" << boundStr
755+
<< "' with trailing characters ('" << boundStr.substr(idx) << "') that could not be parsed";
756+
}
757+
return static_cast<T>(value);
758+
}
759+
734760
std::string mSelectionName;
735761
std::vector<T> mSelectionValues = {}; ///< Threshold values, sorted from most permissive to most restrictive
736762
std::vector<TF1> mSelectionFunctions; ///< TF1 threshold functions (empty for static selections)

0 commit comments

Comments
 (0)