HEPData / hepdata_lib

Library for getting your data into HEPData
https://hepdata-lib.readthedocs.io
MIT License
16 stars 39 forks source link

Support for TMultiGraph and THStack #92

Open zucchett opened 5 years ago

zucchett commented 5 years ago

Support for TMultiGraph could be probably added to the wishlist. Support for TMultiGraph could make importing complex graphs much easier, and avoid painful conversions from TMultiGraph to TGraph in some cases.

AndreasAlbert commented 5 years ago

Hi @zucchett, good point. We should also add the same functionality for THStack and make sure that they work consistently (i.e. similar calling syntax, same output format). The main difference between the two would be that each graph in a TMultiGraph can have an independent set of x values, wheres all histograms in a THStack will use the same x axis binning.

lucasflores commented 4 years ago

Hi All, ATLAS guy here. Has there been anymore development on implementing THStack functionality? Also just to say this library has been great so far, thanks for putting something like this together. -Lucas

AndreasAlbert commented 4 years ago

Hi @lucasflores, thanks for your feedback, glad the library is helping you! So far, I'm not aware of any developments regarding THStack inputs. If you'd be interested to write a function to facilitate this, your contribution would be very welcome and we'd be happy to provide some pointers!

lucasflores commented 4 years ago

Hi @AndreasAlbert , Cool yeah I just wanted to make sure I wasn't redoing work already done :)

So I took a crack at it, pointers would definitely be appreciated. You can take a look at my dev branch in my fork here https://github.com/lucasflores/hepdata_lib/tree/thstack_dev and relevant commit here https://github.com/lucasflores/hepdata_lib/commit/7f1ec164e881cb51381bda9e218c435d1e47d78c. The additions ended up being minimal. It is essentially just a copy paste of read_hist_1d with a few extra lines (it ultimately might be better to just add the few lines to read_hist_1d/2d). Initial tests have this working as expected (and Adding the 2d version in this way is straight forward but I have yet to test it so it has not been added).

So any thoughts as well as maybe some pointers on writing some official tests to add would be appreciated. Since the function is essentially read_hist_1d I would think the testing would really only be extend to making sure we have a THStack and that TH1s are contained inside.

AndreasAlbert commented 4 years ago

Hi @lucasflores, thanks for writing this. Before commenting on tests etc, let me clarify what I think you are doing. IIUC, you are reading the THstack as if it were a single histogram, and the user ends up with one set of y values that represent the sum of all histograms in the stack. Is that correct? In that case, I agree it might most straightforward to simply merge this into read_hist_1d.

I can imagine though that it might also be useful to instead retrieve not just the total, but the individual stacked components separately, which I think would be an easy addition to what you have already written by adding a switch to read_stack_1d. What do you think?

lucasflores commented 4 years ago

Hi @AndreasAlbert , yeah no problem. And Ah no so this does take in a known specified individual hist name that exists in the stack say "h_Triboson3l" as a single background in your stack of backgrounds. You can also just pass in a bogus hist name and the error message will print out all the names of all the hists that exist in the stack in a list. With regards to the sum of the stack It was my understanding from this https://root-forum.cern.ch/t/thstack-and-getbincontent/18967/2 that by default the last histo in a stack should be the sum, so you should be able to get the sum in the same way as is already implemented. That being said an attempt with a stack from my own analysis I couldn't reproduce this with GetStack (no sum TH1 existed) but we might be doing something weird when we first create the stack. So yeah it is probably a good idea to have some functionality to return the sum as well.

rmanzoni commented 2 years ago

Hello! Is TMultiGraph now supported? Thanks!

clelange commented 2 years ago

No, this discussion has not yet resulted in a pull request. We're always open for contributions!