SE treemaker

[katrinamiller]

This is the treemaker to produce the minitrees necessary to create the single electron gain stability plot and the average single electron waveform shape.

[pdeperio]

Thanks @katrinamiller! There's some Codacy failure you can see below, likely due to code style.

Before committing, you can find all Codacy errors with:

flake8 --ignore E501 hax/treemakers/se_treemaker.py

then if you understand all the errors and agree, can fix automatically with:

autopep8 -i --ignore E501 hax/treemakers/se_treemaker.py

[katrinamiller]

OK, I think I've fixed everything except an error Codacy is picking up on: "Unused variable 'class' ". Not sure what to do with this since I need this to define the minitrees.

[feigaodm]

@katrinamiller Thanks! Since nhits_bounds and width_bounds is hard coded anyway, why not put them in IsolatedPeaks class directly? By doing that you will be able to avoid this problem.

[katrinamiller]

I think the original reason for coding it in this manner (this is actually @JelleAalbers code) is because a wider hit cut (10-35 hits) and no width cut is used to calculate the single electron gain. The rest of the parameters, which are extracted from the waveform simulation, use the stricter hit and width cut defined in the SingleElectrons class.

Another option could be to eliminate the cuts inside the minitree completely & instead apply them in the python notebook.

[JelleAalbers]

Thanks for posting the code Katrina. It looks like a mistake in codacy's parsing, since clearly there is no variable called __class__. The idea of separating IsolatedPeaks is indeed that you might want to study isolated peaks without the specific hits and width cut of the SE treemaker; for example, for making figure 1 in https://xe1t-wiki.lngs.infn.it/doku.php?id=xenon:xenon1t:aalbers:single_electron_gain_stability

[tunnell]

Can you add a _ before the function since this is "private" to the treemaker? Or you can move it within the class.

[tunnell]

Also, add a docstring that explains what the function does

[tunnell]

I told Codacy to ignore it. @katrinamiller if you can make the small changes I mentioned in the review, then we can force merge.... though I think I disabled it, but we'll figure it out. It's a fake problem

[tunnell]

If you add #noqa to the line, you should be fine

[tunnell]

Please add a docstring explaining what this class does. Title and few sentences describing it.

[tunnell]

Please add a docstring explaining what this class does. Title and few sentences describing it.

[tunnell]

Also, add a docstring that explains what the function does

[tunnell]

Add a docstring above here to explain what is in this module. Also, the filename should be more explicit: single_electron.py

[tunnell]

How's this different from peak_treemakers.py?

[tunnell]

Move code to within peak_treemakers.py?

[tunnell]

@katrinamiller If you check that this still works for you, then you (or somebody) can merge.

[katrinamiller]

The modified code works for me! Thanks

[pdeperio]

@katrinamiller is this OK to go into standard production now? how does the file size compare to other minitrees, for example Rn220 run 14080 (in bytes):

1068036 171030_0741_Basics.root
1113218 171030_0741_CorrectedDoubleS1Scatter.root
4014866 171030_0741_Corrections.root
677176 171030_0741_DoubleScatter.root
2241335 171030_0741_Extended.root
73322 171030_0741_FlashIdentification.root
216018 171030_0741_Fundamentals.root
2715931 171030_0741_LargestPeakProperties.root
2027886 171030_0741_LoneSignalsPreS1.root
3565778 171030_0741_LoneSignals.root
2219968 171030_0741_PositionReconstruction.root
3410083 171030_0741_Proximity.root
407139 171030_0741_TotalProperties.root

[pdeperio]

Also, what treemaker name would we need to add to cax? SingleElectrons?

[katrinamiller]

@pdeperio this treemaker will make two minitrees - IsolatedPeaks & SingleElectrons. In my scratch directory, both of these combined take up ~ 8 MB of space per run (~6 MB and ~2 MB, respectively).

As suggested above I believe I can make a small change to IsolatedPeaks and remove the SingleElectrons class (which differs from IsolatedPeaks only by applying cuts on number of hits and width) if we are concerned about space or redundancy. I can easily apply these cuts directly in a notebook after the minitrees are created.

I don't think this would cause any issues, maybe @JelleAalbers can confirm this.

[JelleAalbers]

6 MB is relatively large for a minitree (50% larger than corrections), but not a big worry I think. If we run out of space you could consider strengthening the isolation cut or setting a default n_hits cut (though maybe somebody at some point wants to look at multi-hit pileup). Would be nice if this tree were auto-produced by cax!

Sure, you can remove SingleElectrons; it's basically IsolatedPeaks + a cut. You could also convert it to an in-memory minitree (have it load IsolatedPeaks, apply the cut, and return result), but that seems like a bit of work for something that analysts may want to try and customize anyway.

[feigaodm]

Yeah, to make this treemaker more useful for other applications as well, I would suggest to add more variables, like rise_time, range_90p_area, area_fraction_top etc and make the hits bounds less tight. For eg, in my note to distinguish single electron S2s and S1 by ML, I add a few variables there and modified the hit bounds. Can @katrinamiller take a look and made some modification to this treemaker? We can always make the cut tighter later for various analysis.

[JelleAalbers]

Good idea to add rise time and the 90% area width (area_fraction_top is already in). If we keep IsolatedPeaks, it has no cut on n_hits or width at all, so that should work for your analysis too.

[feigaodm]

@JelleAalbers Yeah, it sounds like IsolatedPeaks treemaker is sufficient for both analysis and we don't need SingleElectron one as you pointed out.

[pdeperio]

ok, so I'll wait for additional variables PR before adding IsolatedPeaks to cax.