Skip to content

[TTree] when merging, ignore trees without branches#17652

Merged
pcanal merged 4 commits intoroot-project:masterfrom
ferdymercury:emptytree
Feb 28, 2025
Merged

[TTree] when merging, ignore trees without branches#17652
pcanal merged 4 commits intoroot-project:masterfrom
ferdymercury:emptytree

Conversation

@ferdymercury
Copy link
Copy Markdown
Collaborator

@ferdymercury ferdymercury commented Feb 6, 2025

This Pull request:

Alternative to #17650 as suggested by pcanal

Changes or fixes:

Fixes #14558
Fixes #12510

@ferdymercury ferdymercury requested a review from pcanal as a code owner February 6, 2025 18:14
@ferdymercury ferdymercury marked this pull request as draft February 7, 2025 07:48
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 7, 2025

Test Results

    18 files      18 suites   4d 6h 29m 39s ⏱️
 2 719 tests  2 719 ✅ 0 💤 0 ❌
47 253 runs  47 253 ✅ 0 💤 0 ❌

Results for commit 7f6cce8.

♻️ This comment has been updated with latest results.

@pcanal
Copy link
Copy Markdown
Member

pcanal commented Feb 7, 2025

Part of my confusion is that #12510 is indeed about a file with branches and no entries.
In this case, the problem is not that there is missing branches or missing entries but rather that the first entry (number 0) in the output file is corrupted (has the wrong values).

With the files https://github.com/root-project/root/files/11333378/dummyFiles.zip, we get:

$ root.exe -b -l -q dummyFiles/dummyFile0.root -e 'dummyTree->Scan("*","","",2,0);'

Attaching file dummyFiles/dummyFile0.root as _file0...
(TFile *) 0x22986b0
************************************
*    Row   * index.ind * dummyVari *
************************************
*        0 *         0 * 0.2612867 *
*        1 *         1 * 0.7484587 *
************************************
$ hadd -f dummyFile10.root dummyFiles/dummyFile1.root dummyFiles/dummyFile0.root 
Info in <hadd>: target file: dummyFile10.root
Info in <hadd>: compression setting for all output: 101
hadd Source file 1: dummyFiles/dummyFile1.root
hadd Source file 2: dummyFiles/dummyFile0.root
hadd Target path: dummyFile10.root:/
$ root.exe -b -l -q dummyFile10.root -e 'dummyTree->Scan("*","","",2,0);'

Attaching file dummyFile10.root as _file0...
(TFile *) 0x2f39310
************************************
*    Row   * index.ind * dummyVari *
************************************
*        0 *         0 * 2.13e-306 *
*        1 *         1 * 0.7484587 *
************************************

I.e. Row 0 is wrong.

Nonetheless, it might (or might not) be fixed by this PR, in particular in my last proposal the part:

     if (newtree && nentries == 0)
        continue; // If we already have the structure and we have no entry, save time and skip

i.e. cloning the structure even if there is no entries, might solve the issue.

@ferdymercury ferdymercury changed the title [TTree] when merging, do not clone branch structure if no entries [TTree] when merging, ignore trees without branches Feb 8, 2025
@ferdymercury ferdymercury marked this pull request as ready for review February 8, 2025 15:56
@ferdymercury ferdymercury requested a review from pcanal February 12, 2025 15:46
[TTree] when merging, do not clone branch structure if no entries

Fixes root-project#14558
Fixes root-project#12510

[test] add fully qualified tree

[tree] skip trees with zero branches in static merge

By pcanal

[test] amend tests with cases

[tree] restore prev behavior

[tree] skip "this" when Merge starts with empty-branch tree

as implemented by pcanal

[skip-ci] clarify comments

[tree] early exit if all empty

[test] fix merge substitution
Copy link
Copy Markdown
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@pcanal pcanal merged commit 57425dd into root-project:master Feb 28, 2025
20 of 21 checks passed
@ferdymercury ferdymercury deleted the emptytree branch March 1, 2025 01:24
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.

[ROOT-4716] TTree merging problems when including empty trees Issue with hadd when first file has empty tree

3 participants