search

LDMX-Software / ldmx-sw

The Light Dark Matter eXperiment simulation and reconstruction framework.
https://ldmx-software.github.io
GNU General Public License v3.0
22 stars 20 forks source link

StatBox default in DQM plots #1361

Closed tvami closed 4 months ago

tvami commented 5 months ago

Is your feature request related to a problem? Please describe.

Is there a reason why the statbox is switched off in default for the DQM histograms? If in TBrowser I click on "Options/Statistics" it does not come back. While if we did the other way around (i.e. have statbox on in default) then I can switch it off easily. I can see the argument to not have statbox when we compare plot, but one can easily switch off the statbox in the comparison code. For looking at plots themselves I'd say the statbox add important info.

Describe the solution you'd like Change this to "1" https://github.com/LDMX-Software/ldmx-sw/blob/trunk/Framework/src/Framework/Histograms.cxx#L20

bryngemark commented 5 months ago

off the top of my head i can only come up with, wonder if having a statbox would make the CI gold/new histo overlays harder to read (as sometimes, wtih an automated placement, they may obscure the distributions).

that's not a strong argument for removing this information, though; the real value of the histogram comparisons lies in the KS test. i would rather see that we actually use the Validation module to make to overlays for visual comparison, perhaps with a nice ratio panel too, for Failed/Passed separately, perhaps even importing the failed ones to the PR... doing that (artifact downloading, manipulation and reuploading) within GitHub actions only might be tricky (or something we should escalate to GitHub devs?) but i think it's what's needed to reach the full potential of our PR tests also for the non-expert user.

tvami commented 5 months ago

OK so you'd be open to have it enabled? What do other people thing? @tomeichlersmith @vdutta @EinarElen @tylerhoroho-UVA

I agree with your points in the second block, the TRatioPlot could be a way to go.

tomeichlersmith commented 5 months ago

I'm fine with enabling it :+1: it was introduced before my time and I haven't bothered with it.