Skip to content

Commit d57a264

Browse files
MichaelLettrichshahor02
authored andcommitted
[rANS] Enforce Consistent Histograms
1 parent 477e5e5 commit d57a264

File tree

3 files changed

+28
-3
lines changed

3 files changed

+28
-3
lines changed

DataFormats/Detectors/Common/include/DetectorsCommonDataFormats/EncodedBlocks.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,10 @@ class EncodedBlocks
339339
using dictionaryType = std::variant<rans::RenormedSparseHistogram<source_T>, rans::RenormedDenseHistogram<source_T>>;
340340
#endif
341341

342-
void setHeader(const H& h) { mHeader = h; }
342+
void setHeader(const H& h)
343+
{
344+
mHeader = h;
345+
}
343346
const H& getHeader() const { return mHeader; }
344347
H& getHeader() { return mHeader; }
345348
std::shared_ptr<H> cloneHeader() const { return std::shared_ptr<H>(new H(mHeader)); } // for dictionary creation
@@ -371,6 +374,19 @@ class EncodedBlocks
371374
assert(static_cast<int64_t>(std::numeric_limits<source_T>::min()) <= static_cast<int64_t>(metadata.max));
372375
assert(static_cast<int64_t>(std::numeric_limits<source_T>::max()) >= static_cast<int64_t>(metadata.min));
373376

377+
// check consistency of metadata and type
378+
[&]() {
379+
const int64_t sourceMin = std::numeric_limits<source_T>::min();
380+
const int64_t sourceMax = std::numeric_limits<source_T>::max();
381+
382+
const int64_t mdMin = metadata.min;
383+
const int64_t mdMax = metadata.max;
384+
385+
if ((mdMin < sourceMin) || (mdMax > sourceMax)) {
386+
throw std::runtime_error(fmt::format("value range of dictionary and target datatype are incompatible: target type [{},{}] vs dictionary [{},{}]", sourceMin, sourceMax, mdMin, mdMax));
387+
}
388+
}();
389+
374390
if (ansVersion == ANSVersionCompat) {
375391
rans::DenseHistogram<source_T> histogram{block.getDict(), block.getDict() + block.getNDict(), metadata.min};
376392
return rans::compat::renorm(std::move(histogram), metadata.probabilityBits);

Utilities/rANS/include/rANS/internal/containers/DenseHistogram.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,16 @@ auto DenseHistogram<source_T, std::enable_if_t<sizeof(source_T) <= 2>>::addFrequ
586586
addedHistogramView = trim(addedHistogramView);
587587

588588
if constexpr (std::is_unsigned_v<source_T>) {
589-
LOG_IF(warning, addedHistogramView.getMin() < 0) << fmt::format("trying to add frequencies for a signed symbol to a DenseHistogram of an unsiged type.");
589+
LOG_IF(warning, addedHistogramView.getMin() < 0) << fmt::format("trying to add frequencies of a signed symbol type to a DenseHistogram of an unsiged type.");
590+
}
591+
if constexpr (std::is_signed_v<source_T>) {
592+
const std::ptrdiff_t sourceTypeMax = std::numeric_limits<source_T>::max();
593+
const bool isMinOutOfRange = addedHistogramView.getMin() > sourceTypeMax;
594+
const bool isMaxOutOfRange = addedHistogramView.getMax() > sourceTypeMax;
595+
596+
if (isMaxOutOfRange || isMinOutOfRange) {
597+
LOGP(warning, "trying to add frequencies of an unsigned symbol type to a DenseHistogram of a signed type");
598+
}
590599
}
591600

592601
const auto thisHistogramView = makeHistogramView(this->mContainer);

Utilities/rANS/include/rANS/internal/transform/renorm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ decltype(auto) renorm(histogram_T histogram, Metrics<typename histogram_T::sourc
143143
return nSymbols;
144144
}();
145145

146-
if (nSorted < correctableIndices.size()) {
146+
if ((nSorted < correctableIndices.size()) && (renormingPolicy != RenormingPolicy::ForceIncompressible)) {
147147
std::partial_sort(correctableIndices.begin(), correctableIndices.begin() + nSorted, correctableIndices.end(), [](const auto& a, const auto& b) { return a.second < b.second; });
148148
} else {
149149
std::stable_sort(correctableIndices.begin(), correctableIndices.end(), [](const auto& a, const auto& b) { return a.second < b.second; });

0 commit comments

Comments
 (0)