Skip to content

Conversation

@c-dilks
Copy link
Member

@c-dilks c-dilks commented Aug 28, 2024

Resolves #323

@c-dilks c-dilks linked an issue Aug 28, 2024 that may be closed by this pull request
@c-dilks c-dilks changed the title fix: remove hipo-add fix: rename hipo-add to hipo-merge-histograms and clarify usage Aug 28, 2024
@baltzell
Copy link
Collaborator

maybe best to just remove it altogether?

@c-dilks
Copy link
Member Author

c-dilks commented Aug 29, 2024

maybe best to just remove it altogether?

That was my original idea, but hipo-utils -merge does not actually add histograms (even though it returns exit code 0). On the other hand, hipo-add / hipo-merge-histograms does. So we have:

  • hipo-utils -merge for combining data files (e.g., DST files)
  • hipo-merge-histograms for combining files with histograms

On the other hand, ROOT's hadd does both of these, which is why this behavior confused me.

A better solution would be to have hipo-utils -merge add the histograms correctly, but that's an upstream issue (org.jlab.jnp.hipo4.utils.HipoUtilities, cc @gavalian)

@gavalian
Copy link
Contributor

Merge is a function of HIPO library, which just merges events. However, the hadd is a function of GROOT which has to read and interpret histogram classes and add the content. The HIPO library does not have histogram implementations and can not read and parse the content. That's why this functionality is kept separate.

@baltzell baltzell merged commit ec94673 into development Sep 5, 2024
@baltzell baltzell deleted the 323-hipo-add-is-broken-and-verbose branch September 5, 2024 19:17
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.

hipo-add should give a clear error message when used on data files

4 participants