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
64 stars 268 forks source link

Using monitoring data in cleaning #2331

Closed aleberti closed 1 year ago

aleberti commented 1 year ago

Hi all, as one of the next steps in the development of magic-cta-pipe (see https://github.com/cta-observatory/magic-cta-pipe, used for the MAGIC-LST1 performance paper), I wanted to use as much as possible ctapipe tools, especially in the low level part. In that way, since LST will move to use the DL1 data format (at least if I understood correctly), I wanted to do the same for MAGIC data.

Currently we use the script https://github.com/cta-observatory/magic-cta-pipe/blob/master/magicctapipe/scripts/lst1_magic/magic_calib_to_dl1.py , so I wanted to convert it to use ctapipe-process internally. The MAGIC event source has been already updated to work with ctapipe v0.19, so the only thing to be changed is the cleaning i.e. make it a subclass of ImageCleaner and then configure it in the stage1 configuration file.

However, I realized that with the current ImageCleaner, I cannot use the data within the mon monitoring container. This is used to pass to the cleaning to pass a mask of unsuitable pixels (see https://github.com/cta-observatory/magic-cta-pipe/blob/master/magicctapipe/scripts/lst1_magic/magic_calib_to_dl1.py#L175), later used to e.g. interpolate pixel charges and times.

Is there a way to pass this information to ImageCleaner in the current version? or should this be a feature added to the image cleaner class? I saw that there is an InvalidPixelHandler class but it is used only during the calibration step.

maxnoe commented 1 year ago

This is essentially the same as: https://github.com/cta-observatory/ctapipe/issues/2015#issue-1304899207

aleberti commented 1 year ago

Ok, I see, I was searching through the issues and PRs yesterday but I missed that one. Thanks!

maxnoe commented 1 year ago

Closing as a duplicate, but I will come back to this soon

maxnoe commented 8 months ago

However, I realized that with the current ImageCleaner, I cannot use the data within the mon monitoring container. This is used to pass to the cleaning to pass a mask of unsuitable pixels (see https://github.com/cta-observatory/magic-cta-pipe/blob/master/magicctapipe/scripts/lst1_magic/magic_calib_to_dl1.py#L175), later used to e.g. interpolate pixel charges and times.

@aleberti @Hckjs just mentioned this to me.

I don't think you should interpolate unusable pixels inside the cleaning. in ctapipe we have a InvalidPixelHandler / NeighborAverage that will update the image, which should be called before doing the cleaning. You pass the corrected image / peak time to the cleaning, not do the interpolation inside.

https://github.com/cta-observatory/ctapipe/blob/main/src/ctapipe/image/invalid_pixels.py

aleberti commented 8 months ago

yes, this I know, but it is connected to the Calibrator right? Can I still use it even If I do not want to extract charge and times, since I already have them? I guess so, probably I just need to fill only the DL1 container (I think it is done like this already in ctapipe_io_magic) and it will skip the R1 to DL0 and DL0 to DL1 steps, right? In that case, I think what is needed is to implement an invalid pixel handler based on InvalidPixelHandler, right? Probably the NeighborAverage is already fine, I have to check what is done in the MARS one exactly.

maxnoe commented 8 months ago

Hi, for using this in ctapipe-process with a source that is already providing DL1, that's true. That's a case we hadn't thought about.

You could just call it inside the source or in your own script.

We could also think about moving this invalid pixel handler from the CameraCalibrator to the ImageProcessor.

This might make more sense anyways thinking about procedure: store the raw images without interpolation in a dl0 to dl1a step and then be able to e.g. try out different interpolations later.