-
Notifications
You must be signed in to change notification settings - Fork 53
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
PMT Waveform Simulation #324
Open
atcsutton
wants to merge
12
commits into
ANNIEsoft:Application
Choose a base branch
from
atcsutton:feature/asutton_MCPMTWaveforms
base: Application
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
PMT Waveform Simulation #324
atcsutton
wants to merge
12
commits into
ANNIEsoft:Application
from
atcsutton:feature/asutton_MCPMTWaveforms
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… for fit parameters, general cleanup
…ype with p0 scaled based on the gain data.
…. Also change TGraph pointer to an object.
…/ToolAnalysis into feature/asutton_MCPMTWaveforms
…ly need them to interface with BackTracker. Second, adding the stop_time to the ADCPulse (have version control and default value for backward compatibility). Third, integrating BackTracker. Fourth, allowing for an upstream tool to signal to PhaseIIADCHitFinder, ClusterFinder, and BackTracker that a given Execute step will be skipped. This occurs if there are no MCHits to process or if no good waveforms are produced, and prevents red herring errors from being thrown.
…en averaging them.
This was referenced Jan 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Here's a second try at this PR after some cleanup, extension, and optimization.
The primary change is adding in the PMTWaveformSim tool. Details on the implementation can be found on docDB: https://annie-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=5821. One major change that has been implemented since that presentation is a time optimization. Originally, I was simulating a full 70 us readout window for every hit. That means ~35000 samples, which is very time intensive. Now, I am only simulating up to the latest hit time for each PMT. Adding the waveform simulation cause about 2X more run time compared to just using the true MCHits. However, the ClusterFinder is still much more intensive; needing about 20s to run over a single test file, compared to the 4s that PMTWaveformSim requires.
I have also included some minor changes to downstream tools that allows us to incorporate the simulated waveforms. PhaseIIADCHitFinder needed some specific changes and a new running mode flag. Additionally, I added the ability to skip an execute loop in PhaseIIADCHitFinder, ClusterFinder, and BackTracker. The skip variable is set in the ANNIEEvent by PMTWaveformSim in the case that there are no good MCHits to simulate. Skipping the execute of downstream tools prevents unnecessary error prints from occurring.
The final change is unrelated to the waveform simulation, but is an issue I found during this work. As reported on the last slide of the above docDB, the baseline variance calculation implemented in PhaseIIADCCalibrator::make_calibrated_waveforms_ze3ra_multi is incorrect. Originally, the noise sigma was determined by taking the variance of multiple baseline means. In the case of a single baseline sample this procedure will return a variance of 0. The proper thing to do is to average out the variances of each baseline mean estimation. Additionally, there is a feature which uses the "first baseline" if all of the determined baselines are too noisy. In that case, the original implementation would inadvertently set the baseline to 0.
Breakdown of the 21 files changed:
8 are part of the example ToolChain config
3 are the actual tool (header, source, and readme)
2 are the extension of ADCPulses to record their stop time.
2 are extending BackTracker to handle MC waveforms
3 are changes to PhaseIIADCHitFinder and ClusterFinder to handle MC waveforms
2 are generic changes to Unity and Factory
1 is the PhaseIIADCCalibrator bug fix
Checklist before submitting your PR
new
usage, there is a reason the data must be on the heapnew
there is adelete
, unless I explicitly know why (e.g. ROOT or a BoostStore takes ownership)Additional Material
Attach any validation or demonstration files here. You may also link to relavant docdb articles.