Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add constant charge scaling and pulse shape scaling correction #428

Merged

Conversation

FrancaCassol
Copy link
Collaborator

In this PR I add trialets to the LSTCameraCalibrator and to the LSTCalibrationCalculator in order to give the possibility to:

  1. Scale the integrated charges with constant values different for HG and LG

  2. Scale the integrated charge for the window width on the base of the present reference pulse shape and the ctapipe code used for MC data. This is for testing different integration widths (and eventually pulse shapes). I would like to notice that the present window (8 ns width, 4 ns shift) corresponds to a scaling correction of [1.2032, 0.9486] for [HG, LG]. The LG value is less than one due to the negative undershoot of the present pulse shape (see attached figure), I admit i am not totally sure this is the correct value to be used.

  3. Finally, I add also the possibility to correct the flat-field signal for the integration window before calculating the gain, in this case, due to the large window used (12 ns), the scaling correction is close to 1 : [1.0552, 0.9978] for [HG, LG]

For the moment, the default values of the trailers are such that no scaling is applied.
reference_pulse_shape_MC

@codecov
Copy link

codecov bot commented Jun 1, 2020

Codecov Report

Merging #428 into master will decrease coverage by 0.10%.
The diff coverage is 30.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #428      +/-   ##
==========================================
- Coverage   41.42%   41.32%   -0.11%     
==========================================
  Files          77       77              
  Lines        6336     6371      +35     
==========================================
+ Hits         2625     2633       +8     
- Misses       3711     3738      +27     
Impacted Files Coverage Δ
lstchain/reco/r0_to_dl1.py 64.48% <ø> (ø)
lstchain/calib/camera/calibrator.py 24.52% <18.75%> (-3.68%) ⬇️
lstchain/calib/camera/calibration_calculator.py 28.20% <42.85%> (+0.80%) ⬆️
lstchain/calib/camera/flatfield.py 18.75% <100.00%> (+0.73%) ⬆️
lstchain/calib/camera/pedestals.py 20.00% <100.00%> (+0.89%) ⬆️

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 0755a19...0ee84c0. Read the comment docs.

@moralejo
Copy link
Collaborator

moralejo commented Jun 2, 2020

Thanks for this much needed feature. How is the cross-calibration of high and low gain done in case this correction is activated? Is it done on the already corrected values (for both gains)? Or is the correction done afterwards?

@FrancaCassol
Copy link
Collaborator Author

Thanks for this much needed feature. How is the cross-calibration of high and low gain done in case this correction is activated? Is it done on the already corrected values (for both gains)? Or is the correction done afterwards?

Actually I did not touch the cross-calibration. Could you remember me what did you expect to change?

@moralejo
Copy link
Collaborator

moralejo commented Jun 2, 2020

Thanks for this much needed feature. How is the cross-calibration of high and low gain done in case this correction is activated? Is it done on the already corrected values (for both gains)? Or is the correction done afterwards?

Actually I did not touch the cross-calibration. Could you remember me what did you expect to change?

Ok, so you are applying the ENF method for HG and LG independently, and obtain conversion factors for both, but do not "enforce" the consistency of them, i.e. that for the flatfield events they come up with the same number of p.e. One may argue that in case there is a discrepancy (as was the case before), it is better to have a smooth HG to LG transition, which can be achieved precisely by enforcing this consistency, rather than having a "step" in the change from one to the other. One might either change the LG conv. factor, or the HG factor, or both, to do this cross-calibration.

@FrancaCassol
Copy link
Collaborator Author

Ok, so you are applying the ENF method for HG and LG independently, and obtain conversion factors for both, but do not "enforce" the consistency of them, i.e. that for the flatfield events they come up with the same number of p.e.

Actually at present (after last corrections) HG and LG come out with the same number of photon electrons with the F-factor method. But what is indeed be consolidated is the gain value (which depends on the signal charge and its correction)

One may argue that in case there is a discrepancy (as was the case before), it is better to have a smooth HG to LG transition, which can be achieved precisely by enforcing this consistency, rather than having a "step" in the change from one to the other. One might either change the LG conv. factor, or the HG factor, or both, to do this cross-calibration.

Yes, I agree. Let me verify which step we have now. One could also think to linearly move from one calibration to the other.

@rlopezcoto
Copy link
Contributor

@FrancaCassol @moralejo this implementation will change when moving to ctapipe v0.8, so I propose to accept it as it is now and maybe open an issue about the cross-calibration in the HG/LG transition for the future implementation, how does that sound?

@FrancaCassol
Copy link
Collaborator Author

I agree @rlopezcoto, let's keep it as it is now, we will adapt it to ctapipe 0.8 next week

@moralejo
Copy link
Collaborator

moralejo commented Jul 2, 2020

  1. Scale the integrated charge for the window width on the base of the present reference pulse shape and the ctapipe code used for MC data. This is for testing different integration widths (and eventually pulse shapes). I would like to notice that the present window (8 ns width, 4 ns shift) corresponds to a scaling correction of [1.2032, 0.9486] for [HG, LG]. The LG value is less than one due to the negative undershoot of the present pulse shape (see attached figure), I admit i am not totally sure this is the correct value to be used.

I am indeed afraid it will not be correct for the real data. If you look at slide 7 of Yukiho's presentation linked below you can see that the undershoot is nearly non-existent. Perhaps Yukiho can provide a better curve, can you contact him?
I think we can anyway merge this as Rubén proposes, and continue with it after we move to ctapipe 0.8.

https://indico.cta-observatory.org/event/2853/contributions/24597/attachments/17750/23825/ChargeRecoYukiho20200629.pdf

@rlopezcoto
Copy link
Contributor

@FrancaCassol could you please open an issue to keep this in mind, also with the relevant links/plots? I'll merge this anyways, so we can move forward.

@rlopezcoto rlopezcoto merged commit 4679456 into cta-observatory:master Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants