ANNIEsoft / ToolAnalysis

Other
10 stars 53 forks source link

Event Building LAPPD #216

Closed mnieslony closed 1 year ago

mnieslony commented 1 year ago

The necessary additions for including the LAPPD data stream into the EventBuilding for ANNIE are included.

LAPPD waveforms are saved as full waveforms for the moment until the analysis group has converged on official pulse finding and pedestal settings.

marc1uk commented 1 year ago

Alright, cool. I can't spot anything specifically to delay this (I'm sure you'll be relieved to hear!).... perhaps because it's all well over my head by now. :sweat_smile:

I think the only thing I would say is it feels a bit uncomfortable having this hard-coded, externally derived offset to LAPPD timestamps. I couldn't quite pick up from the README how these numbers are supposed to be generated. I'm not sure this is a good reason to delay the merge, since it works, we want eyes on LAPPD data, and don't have anything better, but perhaps we need to discuss with the LAPPD folks if there's a better approach.

As a very last case the offsets could be stored in the run database, rather than a whole bunch of config files.

mnieslony commented 1 year ago

Hi @marc1uk,

Fair, I created a small github repository where the script for calculating the offsets (and creating some other plots) is located. The link to the repo is included in the README file for the DataDecoderwLAPPD toolchain. I agree that this should probably be automated somehow, but this will require some further discussion with the LAPPD group to find better algorithms/approaches to handle the cases where the offset can't be calculated correctly.