search

cta-observatory / ctapipe_io_lst

ctapipe IO plugin for LST prototype data
BSD 3-Clause "New" or "Revised" License
0 stars 17 forks source link

Empirical calibration correction, re-locate? #89

Open moralejo opened 3 years ago

moralejo commented 3 years ago

https://github.com/cta-observatory/ctapipe_io_lst/blob/67b609ddcd8df495b6a2ac3f7e7fe2801f118106/ctapipe_io_lst/calibration.py#L122-L130

see also https://indico.cta-observatory.org/event/3052/contributions/25956/attachments/18098/24447/CalibrationScalingFactors_5_10_2020.pdf

These correction factors have two components:

  1. one that accounts for the extra "noise" in pulse charge reconstruction, coming from the un-corrected uneven DRS4 sampling, which therefore slightly affects the F-factor method result.
  2. one that corrects for the fact that in the F-factor calibration procedure we integrate 12 samples, whereas for the cosmics we integrate 8.

Since here we are calibrating the waveforms, and we should do it such that the full integration results in the true number of p.e., the correction (2) does not really belong here. That correction depends on the integration window used for calibration and for cosmics, so it should rather be applied at a later stage.

I am NOT proposing to change this immediately, but certainly we should do it before we use this event source regularly with ctapipe's stage1 tool. In that case we can use ctapipe's pulse integration correction to take care of (2).

maxnoe commented 3 years ago

@moralejo These values are not used to change the waveforms, they are only filled into the event.calib.dl1 container to be used by the CameraCalibrator as correction when calculating the image.

moralejo commented 3 years ago

Ah, ok, but as said above part of such factors are integration-details specific, so that part does not belong here.

moralejo commented 3 years ago

Not that it is a problem right now, but it is prone to making mistakes.

FrancaCassol commented 3 years ago

Hi @moralejo,

I agree, the first factor (which by the way depends also from the FF integration window) and the 12 ns correction from the calibration side should actually be already included in the calibration constants, the 8 ns correction of the cosmics should be applied later at integration level. Let take it in mind for next

maxnoe commented 3 years ago

@moralejo The problem right now is that the EventSource is the only place where we can let custom LST configuration enter into the ctapipe tools, until a better setup for monitoring information is available, this needs to be filled by the source

moralejo commented 3 years ago

@maxnoe I am not sure I understood. I would put part (1) of the correction in the calculation of conversion factors, as suggested by Franca. Then for (2) we could adopt, also for real data, ctapipe's pulse integration correction. We only need to provide the pulse shape needed by ctapipe for the correction - perhaps this is what you mean that cannot yet be done?

maxnoe commented 3 years ago

@moralejo No, I just didn't understand your proposal fully.

That will work! The pulse template is part of the Subarray description.

rlopezcoto commented 3 years ago

We only need to provide the pulse shape needed by ctapipe for the correction

If I do not recall wrong, this was the part that was problematic for real data, right @FrancaCassol?

maxnoe commented 3 years ago

Only be cause we couldn't deactivate the correction. This is solved since quite some time and we now provide the correct pulse shape and deactivate the correction.

If I understood correctly, we would now change the corrections to be on and calculate the calibration file accordingly.

moralejo commented 3 years ago

Only be cause we couldn't deactivate the correction. This is solved since quite some time and we now provide the correct pulse shape and deactivate the correction.

If I understood correctly, we would now change the corrections to be on and calculate the calibration file accordingly.

Yes, that is the correct solution I think. But I would do that once we move to using stage1.