[treeplayer] Fix TTreeFormula vector index into vector-of-vectors#22601
Open
guitargeek wants to merge 1 commit into
Open
[treeplayer] Fix TTreeFormula vector index into vector-of-vectors#22601guitargeek wants to merge 1 commit into
guitargeek wants to merge 1 commit into
Conversation
When a vector-type branch was used as the index of a vector-of-vectors
branch, TTreeFormula computed the wrong number of instances and emitted
out-of-bounds errors. For example, with v2 a vector<int> used as an index:
vv1[v2][0] looped over the summed size of all sub-vectors of vv1
instead of the length of v2, printing a spurious extra
instance and "index out of bound" errors.
vv1[0][v2] silently dropped its last instance.
Two issues in the multi-variable-dimension machinery:
1. DefineDimensions registered the inner jagged dimension as a variable
dimension even when the outer dimension is selected through a variable
"gather" index (-2) and the inner dimension is pinned to a constant.
In that configuration the formula does not iterate over the jagged
sub-dimension, so the manager must not sum the physical sub-sizes; the
number of instances is simply the length of the index variable.
2. GetRealInstance performed a premature out-of-bounds check against the
physical sub-size for a variable inner index (-2), using the raw
instance number (the index of the index variable) rather than the
evaluated index. The real bounds are already checked after evaluating
the index variable.
Adds regression tests to treeplayer_regressions.
Closes root-project#19290 (originally JIRA ROOT-8726).
🤖 Done with the help of [Claude Code](https://claude.com/claude-code) (Claude Opus 4.8)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a vector-type branch was used as the index of a vector-of-vectors branch, TTreeFormula computed the wrong number of instances and emitted out-of-bounds errors. For example, with v2 a vector used as an index:
vv1[v2][0] looped over the summed size of all sub-vectors of vv1
instead of the length of v2, printing a spurious extra
instance and "index out of bound" errors.
vv1[0][v2] silently dropped its last instance.
Two issues in the multi-variable-dimension machinery:
DefineDimensions registered the inner jagged dimension as a variable dimension even when the outer dimension is selected through a variable "gather" index (-2) and the inner dimension is pinned to a constant. In that configuration the formula does not iterate over the jagged sub-dimension, so the manager must not sum the physical sub-sizes; the number of instances is simply the length of the index variable.
GetRealInstance performed a premature out-of-bounds check against the physical sub-size for a variable inner index (-2), using the raw instance number (the index of the index variable) rather than the evaluated index. The real bounds are already checked after evaluating the index variable.
Adds regression tests to treeplayer_regressions.
Closes #19290 (originally JIRA ROOT-8726).
🤖 Done with the help of Claude Code (Claude Opus 4.8)