-
Notifications
You must be signed in to change notification settings - Fork 59
PMTWaveformSim tool (re-make of PR #321) #358
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.
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
Conversation
add timing uncertainty map to .h
jminock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to note the following:
- the declaration of new in LoadGeometry.cpp is in the initialization step and the variable needs allocated memory for the duration of its ToolChain's execution (used in CStore). This change does NOT present any memory leaks or takes too much memory.
- PMTWaveformSim.cpp sets a random seed in the initialization step. I never tested using a random seed in ToolAnalysis. Would this seed set the RNG for just this Tool or for whole the ToolChain this Tool is included in? It can stay because I don't have an alternative, and I don't think there are any other Tools that would be affected by this. Please make this addition clear in case a related problem comes up in the future.
I really don't know/haven't tested the scope of RNG seeds. It would be neat if one could set a seed for an entire ToolChain in a configfile but that's outside the scope of this PR. It doesn't present any problems now, but it MAY present a problem in the future, and I think the best way to handle that would be to document it well so IF someone in the future encounters anything related to this, they can trace it back to here and understand what's happening.
| } | ||
|
|
||
| // random seed is grabbed from the system clock | ||
| fRandom.SetSeed(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good to set a seed for the RNG, but I don't know if this scope is the most appropriate. In documentation, please note this line in case a downstream Tool decides to set the RNG and conflicts arise/results from the RNG begin to differ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Andrew had previously set this line to: fRandom = new TRandom3();
Out of an abundance of caution for memory leaks I changed it to .SetSeed(0). I'll update the documentation. It would be interesting to see if this really does change any other downstream tools' seeding (maybe it would just set the seed if not otherwise specified for that given event, then a new seed is grabbed for the next event based on the clock time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend putting in here something along the lines of "CAUTION RNG seed is set in Initialization step of this Tool"
Caution note for seeding
|
README has been updated to note any users that the random seeding in this tool may cause unexpected behavior for downstream tools. |
jminock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thank you! This PR is ready for discussion and merger!
|
Happy to merge once CI finishes |
Describe your changes
Updated modifications of the
PMTWaveformSimtool, originally created by Andrew and opened in PR #321.From his PR:
"Implementation of waveform simulation for tank PMTs. PMTWaveformSim generates PMT waveforms based on the simulated PMT hits. For each MCHit a lognorm waveform is sampled based on fit parameters extracted from SPE waveforms in data. The fit parameters are randomly sampled in a way that conserved the covariance between parameters and captures the random behavior of a PMT's response. A full extended readout window (70 us) is sampled and waveforms from overlapping hits are added. A random baseline between 300 and 350 ADC is added. Random noise is applied, where the noise sigma is sampled from a Gaussian with mean 1 and std dev of 0.25. The baseline and noise envelope values are hard-coded at the moment. Finally, any ADC counts above 4095 are clipped to mimic saturation."
This tool acts to accurately model our PMT response and pass those "simulated" waveforms to the hit finding tools. We can therefore use "Data-like" MC hits the same as the normal Hits (data).
I have made some additional modifications to that tool, namely I have added "reflections" (PMT ringing) to accurately model the entire ADC pulse we see in the data (not just the main peak), and have added realistic time smearing to the MC hits (so that we don't have to rely on the WCSim time smearing optimized for SK).
I have also modified some downstream tools to work with the
PMTWaveformSimtool, hence all the additional modifications. The following is a summary:To take "data-like" MC hits from the waveform simulator:
AssignBunchTimingMCClusterFinderClusterClassifierPhaseIIADCHitFinderI also made a modification to the
LoadGeometrytool that will add the timing uncertainties to the store. This is perhaps the most significant change outside the "scope" of this tool but it won't break anything as it's already reading from the PMT offset file. It is also needed for thePMTWaveformSimtool as it uses these uncertainties to do the time smearing. Also added a working toolchain.Also PS sorry for the generic sounding branch name - I screwed up my initial commit and needed chatgpt to save (re-base) me lol.
Checklist before submitting your PR
newusage, there is a reason the data must be on the heapnewthere is adelete, unless I explicitly know why (e.g. ROOT or a BoostStore takes ownership)Additional Material
The original details on the waveform simulator can be found here. An update on the reflections can be found here.
The script I used to fit the SPE waveform data and generate the
PMTWaveformLognormFit.csvfile that the tool reads from can be found here.