Skip to content

Commit 7cea10c

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)
1 parent ce2ea4c commit 7cea10c

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>
@@ -585,3 +586,78 @@ TEST(TTreeScan, TTreeGetBranchOfFriendTChain)
585586
throw std::runtime_error("Could not retrieve TTreePlayer from main tree!");
586587
}
587588
}
589+
590+
// https://github.com/root-project/root/issues/7844
591+
// TTree::Scan() used to lose precision when printing 64-bit integer branches
592+
// (e.g. ULong64_t): the "lld" column format, when given without an embedded
593+
// column size (i.e. via "colsize=N col=lld"), was not recognized as a
594+
// "long long" modifier because of an off-by-one in the length-modifier
595+
// detection in TTreeFormula::PrintValue. As a consequence the value was
596+
// evaluated and printed as a double, rounding anything above 2^53.
597+
TEST(TTreeScan, ULong64Precision)
598+
{
599+
// The "long long" Scan/Draw column format is evaluated through `long double`
600+
// (see TTreeFormula::PrintValue), so exact 64-bit integer output is only
601+
// possible where `long double` has more mantissa bits than `double`. That is
602+
// the case on x86-64 (80-bit, 64-bit mantissa) but not, e.g., on macOS ARM
603+
// where `long double` is just a 64-bit `double` (53-bit mantissa). Skip the
604+
// exactness check there, since the value genuinely cannot be represented.
605+
if (std::numeric_limits<long double>::digits <= std::numeric_limits<double>::digits)
606+
GTEST_SKIP() << "long double is not wider than double here; the 64-bit value "
607+
"is genuinely unrepresentable and exactness cannot be checked";
608+
609+
// 1617047019150033926 needs 61 bits, so it cannot be represented exactly
610+
// by a double (53-bit mantissa).
611+
constexpr ULong64_t value{1617047019150033926ULL};
612+
613+
constexpr const char *treeName{"tree_7844"};
614+
ROOT::TestSupport::FileRaii fileGuard{"tree_7844.root"};
615+
{
616+
std::unique_ptr<TFile> file{TFile::Open(fileGuard.GetPath().c_str(), "recreate")};
617+
auto tree = std::make_unique<TTree>(treeName, treeName);
618+
619+
ULong64_t x = value;
620+
tree->Branch("x", &x, "x/l");
621+
tree->Fill();
622+
file->Write();
623+
}
624+
625+
std::unique_ptr<TFile> file{TFile::Open(fileGuard.GetPath().c_str())};
626+
auto tree = file->Get<TTree>(treeName);
627+
628+
auto *treePlayer = static_cast<TTreePlayer *>(tree->GetPlayer());
629+
ASSERT_TRUE(treePlayer) << "Could not retrieve TTreePlayer from main tree!";
630+
631+
ROOT::TestSupport::FileRaii redirectFile{"tree_7844_regression_redirect.txt"};
632+
// SetScanFileName() stores the raw pointer, so keep the path string alive.
633+
const std::string redirectPath{redirectFile.GetPath()};
634+
treePlayer->SetScanRedirect(true);
635+
treePlayer->SetScanFileName(redirectPath.c_str());
636+
637+
// Run a Scan with the given option string into the redirect file and return
638+
// its contents.
639+
auto scanToString = [&](const char *option) {
640+
tree->Scan("x:x-1617047019150033925:x-1617047019150033000", "", option);
641+
std::ifstream redirectStream(redirectPath.c_str());
642+
std::stringstream redirectOutput;
643+
redirectOutput << redirectStream.rdbuf();
644+
return redirectOutput.str();
645+
};
646+
647+
const static std::string expectedScanOut{
648+
R"Scan(************************************************************************************
649+
* Row * x * x-1617047019150033925 * x-1617047019150033000 *
650+
************************************************************************************
651+
* 0 * 1617047019150033926 * 1 * 926 *
652+
************************************************************************************
653+
)Scan"};
654+
655+
// The "long long" column format must print the exact 64-bit value, as well as
656+
// the exact result of integer arithmetic with large constants. The column size
657+
// can either be given separately via "colsize=" (so the format passed to
658+
// TTreeFormula::PrintValue is just "lld") or be embedded in the format token
659+
// itself ("21lld"). Only the former triggered the off-by-one in the
660+
// length-modifier detection, but both must yield the exact output.
661+
EXPECT_EQ(scanToString("colsize=21 col=lld:lld:lld"), expectedScanOut);
662+
EXPECT_EQ(scanToString("col=21lld:21lld:21lld"), expectedScanOut);
663+
}

0 commit comments

Comments
 (0)