Add gain selection to calibration chain

[watsonjj]

The gain is now selected in the R1 - DL0 calibration step. All waveforms and images at DL0 and above no longer have a gain channel dimension - they are shape (n_pixels, n_samples).

For the time being, the ImageExtractors work flexibly with waveform shapes (n_channels, n_pixels, n_samples) and (n_pixels, n_samples), but contain deprecation warnings if the former is used.

This is a huge refactoring of ctapipe that has been intended for a long time. It is likely to break a lot of code. The most common break is when the first dimension was manually removed from waveform and image arrays, and can be addressed as follows:

event.dl1.tel[telid].image[0] -> event.dl1.tel[telid].image
event.dl1.tel[telid].pulse_time[0] -> event.dl1.tel[telid].pulse_time
event.dl0.tel[telid].waveform[0] -> event.dl0.tel[telid].waveform

According to the CTA data model, the R1 waveforms should also have gain selection applied, and therefore only have 2 dimensions. This will likely be addressed in a future PR. A possible direction to take in this regard is to move gain selection from the CameraCalibrator to the EventSource. It becomes the EventSource's responsibility to provide properly calibrated and gain-selected waveforms to ctapipe, which is more inline with the intended CTA operation.

[codecov]

Codecov Report

Merging #1095 into master will increase coverage by 0.15%.
The diff coverage is 93.67%.

@@            Coverage Diff             @@
##           master    #1095      +/-   ##
==========================================
+ Coverage   84.31%   84.46%   +0.15%     
==========================================
  Files         182      182              
  Lines       11025    11196     +171     
==========================================
+ Hits         9296     9457     +161     
- Misses       1729     1739      +10

Impacted Files	Coverage Δ
ctapipe/plotting/bokeh_event_viewer.py	90.98% <ø> (ø)	⬆️
ctapipe/image/reducer.py	100% <ø> (ø)	⬆️
ctapipe/plotting/event_viewer.py	0% <ø> (ø)	⬆️
ctapipe/tools/extract_charge_resolution.py	70.96% <0%> (ø)	⬆️
ctapipe/image/muon/muon_diagnostic_plots.py	11.26% <0%> (ø)	⬆️
ctapipe/calib/camera/gainselection.py	96.87% <100%> (ø)	⬆️
ctapipe/calib/camera/tests/test_gainselection.py	100% <100%> (ø)	⬆️
ctapipe/tools/display_dl1.py	90.09% <100%> (ø)	⬆️
ctapipe/tools/display_summed_images.py	91.07% <100%> (ø)	⬆️
ctapipe/image/muon/muon_reco_functions.py	66.66% <100%> (ø)	⬆️
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3d0306...242f65f. Read the comment docs.

[kosack]

I think this looks ok as long as we mention that there will be special calibration runs that are taken where both channels are read out (e.g. no gain selection applied at the R1 level). But since that is a special case, the EventSource can simply write waveforms with the extra dimension - this would have to be processed with a specialized analysis anyhow. But we should still think of that use case. You could even have an option in the SimTelEventSource to disable the gain selection to be able to simulate that case.

[watsonjj]

I think this looks ok as long as we mention that there will be special calibration runs that are taken where both channels are read out

Does this need to be a special case, with an unconventional array shape, or could it be considered as essentially two separate events? With one processed after the other.

[kosack]

Looking a bit more in detail, I'm confused as to why the gain_selection info is stored with the Monitoring data (which is only written around once per second). Don't you need that per event?

[kosack]

to be more clear, event.mon is supposed to be for data that change on a longer timescale than the event (e.g. pedetals that are measured by a multi-event average, or camera temperatures), even though it may be filled per event, it's not intended to be read or written at that rate. So any data that change per event must be contained in the event.tel[x] data itself, not in the monitoring stream.

[kosack]

I think this looks ok as long as we mention that there will be special calibration runs that are taken where both channels are read out

Does this need to be a special case, with an unconventional array shape, or could it be considered as essentially two separate events? With one processed after the other.

I suppose that would work if it's really needed. I think the main use case is just to be able to plot gain ratio curves, so you wouldn't need to run the rest of the Calibrator anyhow, or at least you'd want a specialized version. In any case, you still have the info at the R0 level, so I think it's not an issue.

[watsonjj]

Looking a bit more in detail, I'm confused as to why the gain_selection info is stored with the Monitoring data (which is only written around once per second). Don't you need that per event?

Ah...that is a fair point. However, are there not use-cases where it doesn't change per event? I put it there as I considered it part of the calibration as opposed to connected to a particular data level.

If the DL0 waveforms are read from a file, how are the gain_selection provided? Where is the appropriate place to put this? The R1 container might seem appropriate at first, but this container will be otherwise empty if reading DL0 waveforms. So the DL0 container instead? But then if ctapipe performs the gain selection, the DL0 container is being filled before DL0 waveforms are obtained.

Therefore I chose a location that was not connected to any data level.

[kosack]

Where is the appropriate place to put this

it should go in the same container as the waveforms, since it is connected to it and changes at the same rate.

[kosack]

suggest minor wording change, but otherwise looks good.

[thomasarmstrong]

Looks good to me.