Set the TA pars correctly by TAMakerADCSimpleWindow

[ArturSztuc]

TAMakerADCSimpleWindow did not set TA parameter correctly: so e.g. the channel_start and channel_end were the same, based off the very last TP in the window.

This PR correct this.

Tested by running drunc offline with ADCSimpleWindow with small readout windows, as well as by running emulation on the PD2 data to find some examples of ground shakes -- large ADC, window of 150 ticks, and looking at the hdf5 output. Example plot below.

Trigger candidate generated using emulation on PD2HD TPStream, with Trigger Activities generated using the ADCSimpleWindow algorithm in red boxes (and black dots representing individual TPs), with TA ADC threshold of 10M, and max window size of 150 ticks each. Red windows extracted from the TA objects.

[MRiganSUSX]

Hi @ArturSztuc,
Thanks for spotting this and improving. I have few comments:

• there is no need to initialize several variables if we are later overwriting them (ie ta.time_start is set on line 91 and then changed in the tp loop later); unless we are afraid that the ta.inputs may be empty ?
• this loses the addition of Time-over-Threshold (tot) for the ta.time_end, both in the initial setup and in the tp loop: is that intentional ?
• the tp loop can be made more efficient by using minmax functions, something like this:

if (!ta.inputs.empty()) {
    auto [min_time, max_time] = std::minmax_element(
        ta.inputs.begin(), ta.inputs.end(),
        [](const auto& a, const auto& b) { return a.time_start < b.time_start; });

    auto [min_channel, max_channel] = std::minmax_element(
        ta.inputs.begin(), ta.inputs.end(),
        [](const auto& a, const auto& b) { return a.channel < b.channel; });

    ta.time_start = min_time->time_start;
    ta.time_end = max_time->time_start;
    ta.channel_start = min_channel->channel;
    ta.channel_end = max_channel->channel;

    // Use std::max_element to find peak ADC efficiently
    auto peak_tp = std::max_element(
        ta.inputs.begin(), ta.inputs.end(),
        [](const auto& a, const auto& b) { return a.adc_peak < b.adc_peak; });

    ta.time_peak = peak_tp->time_peak;
    ta.adc_peak = peak_tp->adc_peak;
    ta.channel_peak = peak_tp->channel;
}

what do you think ?

[ArturSztuc]

Pre-setting the min/max elements was needed in my version, otherwise std::max and std::min wouldn't work.

Funnily enough minmax_element is exactly what ChatGPT suggested too (would avoid the pre-setting stuff too). There are other optimisations to be done here, like using std::move for the inputs (to avoid memory copy), but I didn't explore optimising this further - I just copied a version from another algorithm, and decided to play with optimisation later on. I'm happy to delay this PR untill I have more time to play with optimising for computing performance, for now I just wanted it to output something correct for my emulation tests.

As for TOT: this was somewhat intentional. I personally prefer to have TA/TC windows set after time_starts only, since that's how everything is ordered anyway in the DAQ, and then it's the readout-window's job to decide on what raw data window we should actually pull (which would be dependent on drift time, rather than TA length). We have also removed tot from other TAMs because they were causing issues with long TOT TPs.
But we didn't actually decided/converged on what to do here, so it's a bit of a free-for-all until we come up with a specification and officialise it...

[MRiganSUSX]

I don't mind whether this PR gets in - it is an improvement of what it was. Just wanted to point out that there are several places for improvements long term.
If you decide to merge this now, please make a note/Issue to both unify and optimize all of the algos.

Re the ToT and usage for TA/TC ends - this may require a bigger discussion with DS/CoreSw teams as it may actually affect what is read-out. My understanding was that we should be using the ToT for time_ends and instead filter 'bad' TPs (including with long ToTs) upstream. Clearly there is a need for specification, as you said.