Skip to content

[tree] fix I/O rule for vector<T> with nested split source#22375

Open
bendavid wants to merge 2 commits into
root-project:masterfrom
bendavid:fix-19773-iorule-nested-split-source
Open

[tree] fix I/O rule for vector<T> with nested split source#22375
bendavid wants to merge 2 commits into
root-project:masterfrom
bendavid:fix-19773-iorule-nested-split-source

Conversation

@bendavid
Copy link
Copy Markdown
Contributor

@bendavid bendavid commented May 21, 2026

Summary

  • Fixes [tree] missing value in I/O rule for vector<T> with deep source member #19773: when reading a split TTree branch holding vector<T> whose schema-evolution rule sources a member nested inside a struct (e.g. source = "Deep fDeep"), the rule fired but the new top-level target stayed at its default because the on-file staging area was never populated.
  • Detect this case in TBranchElement::InitializeOffsets by also looking up the data member in the parent's on-file staging class. When found, propagate fOnfileObject to the sub-branch, flag it with a new kReadFromStagingArray bit, and re-build its read action sequence.
  • Wrap the sub-branch's actions with UseCacheVectorLoop (new helper TActionSequence::WrapAllActionsWithUseCacheVectorLoop) so the inner action iterates the staging area with the staging element stride and writes at the offset within the staging element — leaving the rule with a properly filled staging area to read from.

Background

This is the spin-off failure noted in #19773: the fix from #19688 / 121dc61 handles a split sub-object with a rule on the value type when the rule's source is a flat member (e.g. fOldMember → fNewMember). When the rule's source is itself a struct member that has been further split on disk (ve99.fDeep.fDeepMember), the existing logic marked the leaf sub-branch missing (because fDeep no longer exists in the user class) and the staging fDeep was never filled before the rule ran.

Test plan

Verified locally with the reproducer from the issue (mini.tar.gz):

Case Before After
Plain object 1 1
Plain vector<T> 1 1
Tree, split-level 0 1 1
Tree, split-level 99 (bug) 0 1

Also verified manually:

  • Multi-element vector (3 elements, distinct values) — all elements read correctly.
  • Multi-member source struct (fA/fB/fC of mixed types, three rule targets) — all values populated.
  • PR [tree] fix TBranchElement for split collection with renamed inner type #19688 RenameSplitCollection regression case (EvolutionStruct_V2::fOldMember → EvolutionStruct_V3::fNewMember over vector<T> with split level 99) still passes.
  • Same-version split read (no rule) still works.

A gtest mirroring the issue reproducer should be added under tree/tree/test/ (alongside evolution.cxx from #19688) — happy to add it in this PR if reviewers prefer.

Some additional context: This affects reading CMS ALCARECO files produced with older (Run 2) CMSSW releases in newer Run 3-era releases which is relevant for some analysis workflows.

🤖 Generated with Claude Code

When reading a split TTree branch holding vector<T>, where T has a schema-
evolution rule whose source member is itself nested inside a struct, the
rule fired but the new top-level member stayed at its default value because
the on-file staging area was never populated.

The leaf sub-branch for the nested source (e.g. ve99.fDeep.fDeepMember)
could not resolve its path in the user (target) class layout, so it was
marked missing and skipped at read time, leaving the staging area used by
the rule uninitialized.

Detect this case in TBranchElement::InitializeOffsets by also looking up
the data member in the parent's on-file staging class. When found,
propagate fOnfileObject to the sub-branch, flag it with the new
kReadFromStagingArray bit, and re-build its read action sequence. The
sequence is then wrapped with UseCacheVectorLoop so the inner action
iterates over the staging area with the staging element stride and writes
at the offset within the staging element, leaving the rule with a properly
filled staging area to read from.

Fixes root-project#19773.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bendavid bendavid requested a review from pcanal as a code owner May 21, 2026 12:19
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bendavid
Copy link
Copy Markdown
Contributor Author

bendavid commented May 21, 2026

CI note: the debian13 / dev=ON, CMAKE_CXX_FLAGS=-Wsuggest-override failure is a build-time race in the cling cxxmodules system (no actual code problem):

[ 56%] Building CXX object io/io/CMakeFiles/RIO.dir/src/TStreamerInfoActions.cxx.o
<<< cling interactive line includer >>>: fatal error: module file '.../RIO.pcm' is out of date and needs to be rebuilt: module file out of date
rootcling: .../SourceLocation.h:341: unsigned int clang::PresumedLoc::getLine() const: Assertion `isValid()' failed.
gmake[2]: *** [tree/ntuple/test/CMakeFiles/SoAFieldDict.dir/build.make:83: tree/ntuple/test/SoAFieldDict.cxx] Error 1

rootcling for SoAFieldDict.cxx ran while RIO.pcm was being rebuilt from this PR's edit to TStreamerInfoActions.h; the stale pcm tripped a clang assertion. The same job passes on master and on other recent PRs. All other Linux/Windows configs (including alma9 modules_off, ubuntu22 Debug, ubuntu2404 Debug, etc.) pass. Let's see if it passes for the next revision with the formatting fixup.

I've pushed a clang-format fixup for the lint job; that workflow run (and the others on the new SHA) is currently in action_required waiting for approval.

@github-actions
Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 11h 44m 1s ⏱️
 3 858 tests  3 857 ✅ 0 💤 1 ❌
77 075 runs  77 074 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit bf98b74.

@bendavid
Copy link
Copy Markdown
Contributor Author

CI on the new SHA (bf98b74) is now green except for one unrelated test:

  • ✅ all macOS jobs, all Windows jobs, all Linux jobs (including the previously-flaky debian13 / -Wsuggest-override)
  • ✅ clang-format, ruff
  • fedora44test-fitpanel-UnitTesting-xvfb (the only failing test in the entire suite)

This fitpanel GUI test is unrelated to this PR (it's an xvfb fit-panel UI test; this PR touches TBranchElement / TStreamerInfoActions I/O). The same test is currently failing on the latest master nightly (run 26263642733) and the recent master commits touching it (b418388 / db0dcbf / 5eaf984 / 7f601d3) appear to be the source. Should resolve on its own once master is fixed; happy to rebase when that lands if helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[tree] missing value in I/O rule for vector<T> with deep source member

2 participants