search

cta-observatory / ctapipe

Low-level data processing pipeline software for CTAO or similar arrays of Imaging Atmospheric Cherenkov Telescopes
https://ctapipe.readthedocs.org
BSD 3-Clause "New" or "Revised" License
62 stars 266 forks source link

Enable processing of interleaved calibration events via the process tool #2522

Closed TjarkMiener closed 2 months ago

TjarkMiener commented 4 months ago

This commit enables the processing of flatfield and sky pedestal events up to dl1-like using the ctapipe-process tool. Code changes are tested with interleaved calibration events from LST-1. I needed to tweak the pointing frame in hdf5eventsource in order to make it work with the standard lstchain R1 interleaved files (not produced with the latest ctapipe version).

Currently only tested with the ImageExtractors (LocalPeakWindowSum and FixedWindowSum), which are the default ones.

FrancaCassol commented 4 months ago

Hi @TjarkMiener, I am not so sure to understand the goal of this code, why do you need a special process for the calibration and flatfield events? (perhaps I missed something ...)

TjarkMiener commented 3 months ago

Ciao @FrancaCassol, following the suggestion from @kosack in #2519, this PR enables the charge integration and arrival time extraction of the calibration data stream. The DL1-like images and peak times are stored following the current ctapipe data model. Those files, which will most likely be stored temporarily, are supposed to be the input of the (TBD - to be developed) tool extracting the cat-B camera calibration coefficients. The advantage would be that we can make use of ctapipe's TableLoader.

FrancaCassol commented 3 months ago

Ciao @FrancaCassol, following the suggestion from @kosack in #2519, this PR enables the charge integration and arrival time extraction of the calibration data stream. The DL1-like images and peak times are stored following the current ctapipe data model. Those files, which will most likely be stored temporarily, are supposed to be the input of the (TBD - to be developed) tool extracting the cat-B camera calibration coefficients. The advantage would be that we can make use of ctapipe's TableLoader.

Hi @TjarkMiener, I see. However, in the case the flatfield and pedestal events are in a file separated by cosmics (which is our case) and if we solve https://github.com/cta-observatory/ctapipe/issues/1836, flatfield and pedestal events could not just be processed with the standard _calibrate_dl1 function? We need just setting the correct configuration cards (as we do for the other cases). The only thing to provide is to select the correct charge extractor for each event_type (which, by the way it seems to me it is missing in this PR?), this is necessary only in the case flatfields and pedestals are in the same file and we want to process them together, otherwise there is no problem.

mexanick commented 3 months ago

Hi @TjarkMiener, I see. However, in the case the flatfield and pedestal events are in a file separated by cosmics (which is our case)

  1. ACADA will provide us separate streams per telescope and per calibration kind
  2. cta-process will work with only particular event types, so even if the calibration events are truly interleaved (or FF and pedestal combined), it won't be a problem.

and if we solve #1836, flatfield and pedestal events could not just be processed with the standard _calibrate_dl1 function?

We want to actually change it a bit and use the TableLoader.

We need just setting the correct configuration cards (as we do for the other cases). The only thing to provide is to select the correct charge extractor for each event_type (which, by the way it seems to me it is missing in this PR?),

The charge extractor type is an input parameter and so far is restricted to two options - LocalPeakWindowSum and FixedWindowSum. The door is open for other extractors in case other telescopes/cameras will need them.

this is necessary only in the case flat fields and pedestals are in the same file and we want to process them together, otherwise there is no problem.

See the part of the answer above.

TjarkMiener commented 2 months ago

Redundant due to #2529