Modification of the FlashFinder algorithm#925
Conversation
…eractions happening a time >8us before a different bigger flash
…der algorithm creates shortened flashes for some interactions that were vetoed before
There was a problem hiding this comment.
Pull request overview
This PR updates the SBND SimpleFlash flash-finding configuration and algorithm to allow creation of a shortened OpFlash when a new flash candidate starts before (and would overlap with) an already-claimed OpFlash, while propagating the shortened integration window into the produced flash time-width (LiteOpFlash_t::time_err → recob::OpFlash).
Changes:
- Add new FHiCL configuration parameters to control “repeated/shortened flash” creation thresholds and timing constraints.
- Extend
SimpleFlashAlgoto optionally shorten a candidate flash’s integration window when it precedes an existing flash and meets PE/timing requirements. - Add extensive inline comments/debug prints describing configuration, vetoing, and flash construction steps.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
sbndcode/OpDetReco/OpFlash/job/sbnd_flashalgo.fcl |
Adds new tunable parameters for repeated/shortened flash behavior. |
sbndcode/OpDetReco/OpFlash/FlashFinder/SimpleFlashAlgo.h |
Adds new configuration-backed member variables for repeated/shortened flash logic. |
sbndcode/OpDetReco/OpFlash/FlashFinder/SimpleFlashAlgo.cxx |
Implements shortened-flash selection logic and additional configuration validation and debug/commentary. |
Comments suppressed due to low confidence (2)
sbndcode/OpDetReco/OpFlash/FlashFinder/SimpleFlashAlgo.cxx:410
- The loop
for(size_t opch=0; opch<max_ch; ++opch)skips the last channel (max_ch), butpe_vis sizedmax_ch+1. This leaves the last channel’s PE potentially negative/unclamped. Iterate up to and includingmax_ch(oropch < pe_v.size()).
// Set negative PE to 0
// Loop over OpChannels
for(size_t opch=0; opch<max_ch; ++opch) {
// Skip unused channels
if(_opch_to_index_v[opch]<0) continue;
//pe_v[opch] -= _pe_baseline_v[_opch_to_index_v[opch]];
if(pe_v[opch]<0) pe_v[opch]=0;
sbndcode/OpDetReco/OpFlash/FlashFinder/SimpleFlashAlgo.cxx:350
- Typo in debug output string: "does not interfare" should be "does not interfere".
if(_debug) {
std::cout << "Flash @ " << min_time + used_period.first * _time_res
<< " => " << min_time + (used_period.first + used_period.second) * _time_res
<< " does not interfare with THIS flash @ " << min_time + start_time * _time_res
<< " => " << min_time + (start_time + integral_ctr) * _time_res << std::endl;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
asanchezcastillo
left a comment
There was a problem hiding this comment.
Overall the changes look good to me. Thanks for having this done, it's a great improvement to the algorithm! I just left a few minor comments, once they're addressed PR can be approved!
…il truncation, added units to the comments, eliminated default value in configuration parameters
Description
Link(s) to docdb describing changes (optional)
Link of the docdb describing the changes: docdb-46414
Relevant PR links (optional)
This PR does not require merging another PR before
Development
Checklist
Reviewers,AssigneesDevelopement