Skip to content

Commit 729b3e8

Browse files
committed
[treeplayer] Fix precision loss in TTree::Scan for 64-bit integers
The `"lld"` column format passed without an embedded size (e.g. via `"colsize=N col=lld"`) was not recognized as a `"long long"` modifier in **TTreeFormula::PrintValue**, so 64-bit integer branches (e.g. `ULong64_t`) were evaluated as double and rounded above 2^53. This commit fixes a off-by-one problem in the length-modifier detection to address the issue, adding also a regression test. Closes #7844. 🤖 Done with the help of [Claude Code](https://claude.com/claude-code) (Claude Opus 4.8) (cherry picked from commit 7cea10c)
1 parent 64940f8 commit 729b3e8

2 files changed

Lines changed: 78 additions & 2 deletions

File tree

tree/treeplayer/src/TTreeFormula.cxx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5052,12 +5052,12 @@ char *TTreeFormula::PrintValue(Int_t mode, Int_t instance, const char *decform)
50525052
Ssiz_t len = strlen(decform);
50535053
Char_t outputSizeLevel = 1;
50545054
char *expo = nullptr;
5055-
if (len>2) {
5055+
if (len > 1) {
50565056
switch (decform[len-2]) {
50575057
case 'l':
50585058
case 'L': {
50595059
outputSizeLevel = 2;
5060-
if (len>3 && tolower(decform[len-3])=='l') {
5060+
if (len > 2 && tolower(decform[len - 3]) == 'l') {
50615061
outputSizeLevel = 3;
50625062
}
50635063
break;

tree/treeplayer/test/regressions.cxx

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "gtest/gtest.h"
2121

22+
#include <limits>
2223
#include <string>
2324
#include <vector>
2425
#include <fstream>
@@ -668,3 +669,78 @@ TEST(TTreeScan, TTreeGetBranchOfFriendTChain)
668669
throw std::runtime_error("Could not retrieve TTreePlayer from main tree!");
669670
}
670671
}
672+
673+
// https://github.com/root-project/root/issues/7844
674+
// TTree::Scan() used to lose precision when printing 64-bit integer branches
675+
// (e.g. ULong64_t): the "lld" column format, when given without an embedded
676+
// column size (i.e. via "colsize=N col=lld"), was not recognized as a
677+
// "long long" modifier because of an off-by-one in the length-modifier
678+
// detection in TTreeFormula::PrintValue. As a consequence the value was
679+
// evaluated and printed as a double, rounding anything above 2^53.
680+
TEST(TTreeScan, ULong64Precision)
681+
{
682+
// The "long long" Scan/Draw column format is evaluated through `long double`
683+
// (see TTreeFormula::PrintValue), so exact 64-bit integer output is only
684+
// possible where `long double` has more mantissa bits than `double`. That is
685+
// the case on x86-64 (80-bit, 64-bit mantissa) but not, e.g., on macOS ARM
686+
// where `long double` is just a 64-bit `double` (53-bit mantissa). Skip the
687+
// exactness check there, since the value genuinely cannot be represented.
688+
if (std::numeric_limits<long double>::digits <= std::numeric_limits<double>::digits)
689+
GTEST_SKIP() << "long double is not wider than double here; the 64-bit value "
690+
"is genuinely unrepresentable and exactness cannot be checked";
691+
692+
// 1617047019150033926 needs 61 bits, so it cannot be represented exactly
693+
// by a double (53-bit mantissa).
694+
constexpr ULong64_t value{1617047019150033926ULL};
695+
696+
constexpr const char *treeName{"tree_7844"};
697+
ROOT::TestSupport::FileRaii fileGuard{"tree_7844.root"};
698+
{
699+
std::unique_ptr<TFile> file{TFile::Open(fileGuard.GetPath().c_str(), "recreate")};
700+
auto tree = std::make_unique<TTree>(treeName, treeName);
701+
702+
ULong64_t x = value;
703+
tree->Branch("x", &x, "x/l");
704+
tree->Fill();
705+
file->Write();
706+
}
707+
708+
std::unique_ptr<TFile> file{TFile::Open(fileGuard.GetPath().c_str())};
709+
auto tree = file->Get<TTree>(treeName);
710+
711+
auto *treePlayer = static_cast<TTreePlayer *>(tree->GetPlayer());
712+
ASSERT_TRUE(treePlayer) << "Could not retrieve TTreePlayer from main tree!";
713+
714+
ROOT::TestSupport::FileRaii redirectFile{"tree_7844_regression_redirect.txt"};
715+
// SetScanFileName() stores the raw pointer, so keep the path string alive.
716+
const std::string redirectPath{redirectFile.GetPath()};
717+
treePlayer->SetScanRedirect(true);
718+
treePlayer->SetScanFileName(redirectPath.c_str());
719+
720+
// Run a Scan with the given option string into the redirect file and return
721+
// its contents.
722+
auto scanToString = [&](const char *option) {
723+
tree->Scan("x:x-1617047019150033925:x-1617047019150033000", "", option);
724+
std::ifstream redirectStream(redirectPath.c_str());
725+
std::stringstream redirectOutput;
726+
redirectOutput << redirectStream.rdbuf();
727+
return redirectOutput.str();
728+
};
729+
730+
const static std::string expectedScanOut{
731+
R"Scan(************************************************************************************
732+
* Row * x * x-1617047019150033925 * x-1617047019150033000 *
733+
************************************************************************************
734+
* 0 * 1617047019150033926 * 1 * 926 *
735+
************************************************************************************
736+
)Scan"};
737+
738+
// The "long long" column format must print the exact 64-bit value, as well as
739+
// the exact result of integer arithmetic with large constants. The column size
740+
// can either be given separately via "colsize=" (so the format passed to
741+
// TTreeFormula::PrintValue is just "lld") or be embedded in the format token
742+
// itself ("21lld"). Only the former triggered the off-by-one in the
743+
// length-modifier detection, but both must yield the exact output.
744+
EXPECT_EQ(scanToString("colsize=21 col=lld:lld:lld"), expectedScanOut);
745+
EXPECT_EQ(scanToString("col=21lld:21lld:21lld"), expectedScanOut);
746+
}

0 commit comments

Comments
 (0)