Skip to content

Conversation

@S81D
Copy link
Collaborator

@S81D S81D commented Sep 8, 2025

Describe your changes

Updated modifications of the PMTWaveformSim tool, 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 PMTWaveformSim tool, hence all the additional modifications. The following is a summary:

To take "data-like" MC hits from the waveform simulator:

  • AssignBunchTimingMC
  • ClusterFinder
  • ClusterClassifier
  • PhaseIIADCHitFinder

I also made a modification to the LoadGeometry tool 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 the PMTWaveformSim tool 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

  • [No - see above for rationale] This PR implements a single change (one new/modified Tool, or a set of changes to implement one new/modified feature)
  • This PR alters the minimum number of files to affect this change
  • If this PR includes a new Tool, a README and minimal demonstration ToolChain is provided
  • If a new Tool/ToolChain requires model or configuration files, their paths are not hard-coded, and means of generating those files is described in the readme, with examples provided on /pnfs/annie/persistent
  • For every new usage, there is a reason the data must be on the heap
  • For every new there is a delete, 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.csv file that the tool reads from can be found here.

@S81D
Copy link
Collaborator Author

S81D commented Sep 8, 2025

This PR should be merged before #329 and #330 as those PRs rely on the waveform simulator tool. I would have included those tools' changes here but it's easier to leave them open as separate PRs. Sorry again for the long PR.

add timing uncertainty map to .h
Copy link
Collaborator

@jminock jminock left a 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:

  1. 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.
  2. 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);
Copy link
Collaborator

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

Copy link
Collaborator Author

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).

Copy link
Collaborator

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
@S81D
Copy link
Collaborator Author

S81D commented Sep 9, 2025

README has been updated to note any users that the random seeding in this tool may cause unexpected behavior for downstream tools.

Copy link
Collaborator

@jminock jminock left a 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!

@brichards64
Copy link
Contributor

Happy to merge once CI finishes

@brichards64 brichards64 merged commit 7803e72 into ANNIEsoft:Application Sep 19, 2025
1 check passed
@S81D S81D mentioned this pull request Sep 23, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants