Fix PMTWaveformSim event skipping #366
Merged
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
If there are no MCHits to reconstruct a waveform, PMTWaveformSim skipped the event and told other downstream tools to do the same. It was originally implemented to be computationally efficient as to not to simulate all waveforms. It also avoided crashing the PhaseIIADCHitFinder tool; it wasn't handling blank waveform entries well since that doesn't exist in the data.
We noted that ultimately we would have to modify the waveform simulator at some point for the efficiency / cross section analyses since we need those "blank" entries.
My workaround is that if there are no MCHits in an event, rather than skipping the event all together we simulate a "minimal" waveform in one of the dead PMT channels (ID 333) so that the hit finding tool is happy and doesn't crash. The minimal waveform won't register a hit as:
The resulting event information is passed to other tools even though no hits have been registered (like the data) and no downstream tools are skipped.
I also removed logic from the downstream tools that checked whether they should skip the event. This was added previously purely to not crash when loading in the waveform simulated hits. It can be removed as that condition is no longer set by waveform sim.
One of the tools (EventSelector) also needs its skip logic adjusted, but I have added a commit to the outstanding PR #330 to take care of this.
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
Attach any validation or demonstration files here. You may also link to relavant docdb articles.