root icon indicating copy to clipboard operation
root copied to clipboard

[ntuple] Add support for truncated single-precision float columns

Open silverweed opened this issue 1 year ago • 3 comments

This Pull request:

adds a new ColumnType Real32Trunc, that stores a real value as a tightly-packed, IEEE-754 single precision float using less than 32 bits. The missing bits are simply truncated from the mantissa, causing the value to be rounded towards zero. The valid range of bit widths is [10, 31] inclusive.

This is the first ColumnType with a variable bit width, therefore it requires some extra handling on the RColumnElement and RField side, but it uses a single type id on disk (0x1D) and a single enum value EColumnType::kReal32Trunc.

The way to use it is by calling the new SetTruncated(nBits) method on RField<float>.

Checklist:

  • [x] tested changes locally
  • [x] updated the docs (if necessary)

silverweed avatar Aug 08 '24 11:08 silverweed

Test Results

    13 files      13 suites   3d 4h 43m 28s :stopwatch:  3 029 tests  3 029 :white_check_mark: 0 :zzz: 0 :x: 33 856 runs  33 856 :white_check_mark: 0 :zzz: 0 :x:

Results for commit fa0d894a.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Aug 08 '24 13:08 github-actions[bot]

I left some high-level comments, but in general the PR is really hard to read and review because there are unrelated changes (both using namespace and formatting fixes) packed in the individual commits. The first commit also still has traces of using multiple column types (kReal32TruncBegin) that get subsequently removed, which makes it really hard to follow unless reviewing the entire +525 -100 change in one go. If possible, it would be really appreciated to help reviewers reading through the code...

I'll try collecting all the aesthetic-only changes into a separate PR and rework some of the intermediate commits. I'm changing this to a draft until then.

silverweed avatar Aug 14 '24 14:08 silverweed

@hahnjo clang-format PR: #16237

silverweed avatar Aug 14 '24 14:08 silverweed