Move `InvalidPixelHandler` from `CameraCalibration` to `ImageProcessor`?

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

Move `InvalidPixelHandler` from `CameraCalibration` to `ImageProcessor`? #2518

Open maxnoe opened 4 months ago

maxnoe commented 4 months ago

Brought up in a discussion with @aleberti in this issue here: #2331

We added the InvalidPixelHandler as last step of CameraCalibrator when doing the dl0 → dl1 step.

That prevents applying it on input data that does not contain DL0 (e.g. DL1 images loaded with the HDF5EventSource or a custom source as is the case for the MAGIC plugin).

I think we could move the InvalidPixelHandler application from the CameraCalibrator to the ImageProcessor to enable applying it also on DL1 input.

@kosack what do you think?

kosack commented 4 months ago

Yes, it makes sense to have it in ImageProcessor, since that is generally where you want to avoid invalid pixels, so ok by me.

maxnoe commented 2 months ago

I tried to implement this today, but ran into some conceptual issues:

The broken pixels are per channel information
selected_gain_channel is only stored at dl0, not at dl1
The ImageProcessor currently works without needing dl0 information, it is purely dl1
Files containing only dl1 will not contain this information, do they currently contain the broken pixel monitoring data

These are the reasons, why the current InvalidPixelHandler is somewhat tied to the DL0 → DL1 step and is thus in CameraCalibrator.

Also: is_valid is currently a single bool, it should probably be a boolean mask for each pixel, same shape as image itself.