[treeplayer] Fix precision loss in TTree::Scan for 64-bit integers#22600
Conversation
Test Results 21 files 21 suites 3d 7h 17m 18s ⏱️ Results for commit 570f2e1. ♻️ This comment has been updated with latest results. |
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 root-project#7844. 🤖 Done with the help of [Claude Code](https://claude.com/claude-code) (Claude Opus 4.8)
Thanks for the review! I'm fine to keep the issue open even after merging this PR, but I think if the proper fix for all platforms will be much more than a few lines change we should also consider to just error out and declare this a "known limitation", as I also argued in this comment #22600 (comment). But let's see, maybe supporting |
| if (std::numeric_limits<long double>::digits <= std::numeric_limits<double>::digits) | ||
| GTEST_SKIP() << "long double is not wider than double here; the 64-bit value " | ||
| "is genuinely unrepresentable and exactness cannot be checked"; | ||
|
|
There was a problem hiding this comment.
We should consider lifting this restriction by adding explicit support for unsigned long long inside TTreeFormula.
pcanal
left a comment
pcanal
left a comment
There was a problem hiding this comment.
It is a necessary change. Although the test skip may indicate need for further refinement.
As far as I know there should not be any issue with
If we were to add this we would need to "only" print it if the value overflows (i.e. it works in most cases). |
4 participants
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 (Claude Opus 4.8)