Closed adam-abed-abud closed 7 months ago
Closed by #87
This is not reolved by #87. This is still a pending issue because the ADC peak time stored in the TP is wrongly calculated. See here for example https://github.com/DUNE-DAQ/fdreadoutlibs/blob/develop/src/wib2/WIB2FrameProcessor.cpp#L515 Ivana is working on fix for this on the naive TPG implementation.
Hello, A quick update, I have a branch that fixes the peak ADC and peak time calculation for the TriggerPrimitive object in the SWTPG. https://github.com/DUNE-DAQ/fdreadoutlibs/tree/hristova/hit_peak_adc_and_time_fix
As described in this issue, I noticed that there is a second part, which refers to restoring total charge. I haven't done that part yet, but I will have a look next.
Thanks, Ivana
I've updated the branch adc_peak_feature with the validated changes for the ADC peak and peak time calculation. The changes affect the TPG alogrithms (and hit extraction) impemented in AVX, NAIVE and WIBEthFrameProcessor.
As discussed at the TPG technical meeting, the changes in this branch allow to cover all edge cases, except for overflows of the hit parameters, these possible overflows are currently not handled. In case there is a need to handle overflows (e.g. the hit charge), it would be good to discuss what strategy to apply.
In order to finalise these changes, it was useful to create pattern generation app and a pattern validation script, which are being discussed in the other issue, 145.
Hi Ivana, thanks for the update. I will look into the details of the code changes but as discussed last Wednesday the branch aabedabu/adc_peak_feature
should only serve as a playground and you should use instead the branch aabedabu/feature_peak_time_and_adc
for the changes related to this feature. This is because the current branch that you have modified contains features and changes NOT related to the peak adc and peak time such as for example:
get_hits_dest32u
and get_hits_dest16i() I suggest to make all the changes related to the emulators in a different branch so we merge those first to make the merging process simpler and review easier.
Fixed with PR #154
In the current implementation of the SWTPG the time of the peak of the trigger primitive is calculated as follows
trigprim.time_peak = (tp_t_begin + tp_t_end) / 2
and the ADC peak is calculated with
trigprim.adc_peak = hit_charge[i] / 20
It would make more sense to calculate the time of the peak and the corresponding ADC peak value during the calculation of the TP properties in the AVX code.
Also, restore the original value of the total charge when constructing the TriggerPrimitive object. This is due to the left shift operation that effectively divides the charge by a fixed amount. Add something like this for fixing the problem:
trigprim.adc_integral = hit_charge[i]*m_wib2_frame_handler->m_tpg_processing_info->multiplier