Skip to content

hadd with -n option misses some content #9022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
will-cern opened this issue Sep 22, 2021 · 4 comments · May be fixed by #18679
Open

hadd with -n option misses some content #9022

will-cern opened this issue Sep 22, 2021 · 4 comments · May be fixed by #18679
Assignees
@pcanal @silverweed
Labels
bug in:I/O priority:critical

Comments

@will-cern
Copy link
Contributor

I haven't had a chance to look into the hadd code (TFileMerger) but in 6.24/04 I the following problem. Create two files like this:

{
  TFile f1("f1.root","RECREATE");
  TH1D h("h1","h1",10,0,10);
  h.Write();
  f1.Close();
  TFile f2("f2.root","RECREATE");
  TH1D h2("h2","h2",10,0,10);
  h2.Write();
  f2.Close();
}

now if we hadd these as normal (hadd merge.root f1.root f2.root) we get both histograms:

TFile**         merge.root      
 TFile*         merge.root      
  KEY: TH1D     h1;1    h1
  KEY: TH1D     h2;1    h2

but do this with the -n 1 option I only get h1:

TFile**         merge.root      
 TFile*         merge.root      
  KEY: TH1D     h1;1    h1
@jblomer jblomer self-assigned this Sep 23, 2021
@jblomer
Copy link
Contributor

jblomer commented Sep 23, 2021

Additional observation: -n1 is different from -n 1. I can reproduce the problem with -n 1 on master.

@jblomer
Copy link
Contributor

jblomer commented Oct 5, 2021

Option parsing in hadd demands a space between -n and the value. The value is passed on to TFileMerger::SetMaxOpenedFiles() which in turn sets it to a minimum value of fMaxOpenedFiles = 2.

From my tests, it appears that TFileMerger merges the first fMaxOpenedFiles - 1 input files, which is probably not the desired behavior.

@jblomer
Copy link
Contributor

jblomer commented Oct 5, 2021

I'm reassigning to Philippe. I think it boils down to this: when processing the excess files, TFileMerger switches to the incremental mode. In this mode, TFileMerger::MergeOne sets canBeFound = true, which in turn at the end of MergeOne skips the if block commented with // Don't write the partial result for TTree and TH1. Perhaps canBeFound should not be true for a histogram that is not (yet) found in the target file?

@jblomer jblomer assigned pcanal and unassigned jblomer Oct 5, 2021
@pcanal
Copy link
Member

pcanal commented Jan 10, 2025

@silverweed Can you check this issue?

ferdymercury added a commit to ferdymercury/root that referenced this issue May 9, 2025
@ferdymercury
[io] don't miss writing a histogram that is only in a last file with …
5111f69 
…option -n 2

Fixes root-project#9022
@ferdymercury ferdymercury linked a pull request May 9, 2025 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pcanal pcanal

@silverweed silverweed

Labels
bug in:I/O priority:critical
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[io] don't miss writing a histogram that is only in a last file with option -n 2 ferdymercury/root
4 participants
@jblomer @pcanal @silverweed @will-cern