Skip to content

Commit

Permalink
[ntuple] fix ValueRange being always non-null on disk
Browse files Browse the repository at this point in the history
  • Loading branch information
silverweed committed Sep 10, 2024
1 parent 51b1ea3 commit ae7a8ee
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 8 deletions.
2 changes: 1 addition & 1 deletion tree/ntuple/v7/inc/ROOT/RColumn.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public:
assert(fElement);
return fElement->GetBitsOnStorage();
}
std::pair<double, double> GetValueRange() const
std::optional<std::pair<double, double>> GetValueRange() const
{
assert(fElement);
return fElement->GetValueRange();
Expand Down
5 changes: 3 additions & 2 deletions tree/ntuple/v7/inc/ROOT/RColumnElementBase.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <cstddef> // for std::byte
#include <cstdint>
#include <memory>
#include <optional>
#include <string>
#include <type_traits>
#include <typeinfo>
Expand Down Expand Up @@ -55,7 +56,7 @@ protected:
std::size_t fSize;
std::size_t fBitsOnStorage;
/// This is only meaningful for column elements that support it (e.g. Real32Quant)
std::pair<double, double> fValueRange = {0, 0};
std::optional<std::pair<double, double>> fValueRange = std::nullopt;

explicit RColumnElementBase(std::size_t size, std::size_t bitsOnStorage = 0)
: fSize(size), fBitsOnStorage(bitsOnStorage ? bitsOnStorage : 8 * size)
Expand Down Expand Up @@ -110,7 +111,7 @@ public:

std::size_t GetSize() const { return fSize; }
std::size_t GetBitsOnStorage() const { return fBitsOnStorage; }
std::pair<double, double> GetValueRange() const { return fValueRange; }
std::optional<std::pair<double, double>> GetValueRange() const { return fValueRange; }
std::size_t GetPackedSize(std::size_t nElements = 1U) const { return (nElements * fBitsOnStorage + 7) / 8; }
}; // class RColumnElementBase

Expand Down
1 change: 0 additions & 1 deletion tree/ntuple/v7/src/RColumnElement.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -288,4 +288,3 @@ void ROOT::Experimental::Internal::BitPacking::UnpackBits(void *dst, const void
assert(prevWordLsb == 0);
assert(dstIdx == count);
}

6 changes: 4 additions & 2 deletions tree/ntuple/v7/src/RColumnElement.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,8 @@ public:
void Pack(void *dst, const void *src, std::size_t count) const final

Check warning on line 857 in tree/ntuple/v7/src/RColumnElement.hxx

View workflow job for this annotation

GitHub Actions / alma8 LLVM_ENABLE_ASSERTIONS=On

‘void {anonymous}::RColumnElementQuantized<T>::Pack(void*, const void*, std::size_t) const [with T = double]’ declared ‘static’ but never defined [-Wunused-function]
{
auto quantized = std::make_unique<Quantize::Quantized_t[]>(count);
const auto [min, max] = fValueRange;
assert(fValueRange);
const auto [min, max] = *fValueRange;
Quantize::QuantizeReals(quantized.get(), reinterpret_cast<const float *>(src), count, min, max, fBitsOnStorage);
ROOT::Experimental::Internal::BitPacking::PackBits(dst, quantized.get(), count, sizeof(Quantize::Quantized_t),
fBitsOnStorage);
Expand All @@ -866,7 +867,8 @@ public:
void Unpack(void *dst, const void *src, std::size_t count) const final

Check warning on line 867 in tree/ntuple/v7/src/RColumnElement.hxx

View workflow job for this annotation

GitHub Actions / alma8 LLVM_ENABLE_ASSERTIONS=On

‘void {anonymous}::RColumnElementQuantized<T>::Unpack(void*, const void*, std::size_t) const [with T = double]’ declared ‘static’ but never defined [-Wunused-function]
{
auto quantized = std::make_unique<Quantize::Quantized_t[]>(count);
const auto [min, max] = fValueRange;
assert(fValueRange);
const auto [min, max] = *fValueRange;
ROOT::Experimental::Internal::BitPacking::UnpackBits(quantized.get(), src, count, sizeof(Quantize::Quantized_t),
fBitsOnStorage);
Quantize::UnquantizeReals(reinterpret_cast<float *>(dst), quantized.get(), count, min, max, fBitsOnStorage);
Expand Down
4 changes: 2 additions & 2 deletions tree/ntuple/v7/test/ntuple_packing.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ TEST(Packing, OnDiskEncoding)
*e->GetPtr<ClusterSize_t>("index32") = 39916801; // 0x0261 1501
*e->GetPtr<ClusterSize_t>("index64") = 0x0706050403020100L;
*e->GetPtr<float>("float32Trunc") = -3.75f; // 1 10000000 11100000000000000000000 == 0xC0700000
*e->GetPtr<float>("float32Quant") = 0.69f; // quantized to 88 == 0b1011000
*e->GetPtr<float>("float32Quant") = 0.69f; // quantized to 88 == 0b1011000
e->GetPtr<std::string>("str")->assign("abc");

writer->Fill(*e);
Expand Down Expand Up @@ -647,9 +647,9 @@ TEST(Packing, Real32Quant)
constexpr auto kBitsOnStorage = 10;
RColumnElement<float, EColumnType::kReal32Quant> element;
element.SetBitsOnStorage(kBitsOnStorage);
element.SetValueRange(-5.f, 5.f);
element.Pack(nullptr, nullptr, 0);
element.Unpack(nullptr, nullptr, 0);
element.SetValueRange(-5.f, 5.f);

float f1 = 3.5f;
unsigned char out[8];
Expand Down

0 comments on commit ae7a8ee

Please sign in to comment.