Modification of the FlashFinder algorithm

[CarlosNeutrino]

Description

• This PR includes small changes in the FlashFinder algorithm
• It includes the possibility of creating a shortened OpFlash when a flash candidate occurs before an already defined OpFlash
• The time difference between the shortened OpFlash and the later one has to be at least of 0.5μs
• The time-width of the flashes is saved as an attribute to the recob::OpFlash class. Its value will be 4μs for non-shortened flash and a smaller value for the newly added flashes
• The modules that are affected are reco1:opflashtpc0:SBNDFlashFinder and reco1:opflashtpc1:SBNDFlashFinder
• Details and validation plots can be found in the linked docdb

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

• This PR raises this different issue on the OptT0 calculation: Issue-638. The OptT0 calculation needs to take into account the possibility that a flash might be shortened (its width can be less than 8μs).

Checklist

• Added at least 1 label from available labels.
• Assigned at least 1 reviewer under Reviewers,
• Assigned all contributers including yourself under Assignees
• Linked any relevant issues under Developement
• Does this PR affect CAF data format? If so, please assign a CAF maintainer (PetrilloAtWork or JosiePaton) as additional reviewer.
• Does this affect the standard workflow?
• Is this PR a patch for the ongoing production? If so, separate PR must also be made for production/v10_06_00 branch!

[Copilot]

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 SimpleFlashAlgo to 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), but pe_v is sized max_ch+1. This leaves the last channel’s PE potentially negative/unclamped. Iterate up to and including max_ch (or opch < 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]

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!

[fjnicolas]

Hi @CarlosNeutrino, thank you so much for the PR! This is a very nice improvement over the existing code (and thank you so much for adding all the comments to the existing codes, it improved a lot the readability for future contributors!).

I left three minor comments that could help saving some computing time, otherwise, happy to approve!

[fjnicolas]

For the sake of keeping the nice documentaiton you've been adding to the code, it is Total, not per PMT.

Suggested change

	- _min_pe_flash: minimum PE to declare a flash: TOTAL OR PER PMT?
	- _min_pe_coinc: minimum PE in coincidence window to declare a flash candidate: TOTAL OR PER PMT?
	- _min_pe_flash: minimum PE to declare a flash: TOTAL
	- _min_pe_coinc: minimum PE in coincidence window to declare a flash candidate: TOTAL

[fjnicolas]

The division (size_t)(std::ceil(_min_time_before/_time_res))) converting ns to bin indices it's constant throughout the algorithm but it's repeated multiple times within the two "for" loops. I recommend declaring a variable at the beginning to safe some computing time.

[fjnicolas]

This variable is also constant
size_t gap_bins = (size_t)(std::ceil(_time_dif_flash_before/_time_res));
Also recommend declaring it at the beginning to avoid computing the same value multiple times.