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 subarray to calibration class #458

Merged

Conversation

FrancaCassol
Copy link
Collaborator

This is a first fix for the camera calibration, that permit to apply the calibration
(other fixes related to the calibration containers and calibration calculation can come in a second PR)

@rlopezcoto
Copy link
Contributor

@FrancaCassol now that you are at it working with the calibrator.py, the self included in the print in:

print(f"Problem in reading calibration file {self.calibration_path}")

will not work because this function is outside of the LSTCameraCalibrator class, right?

@FrancaCassol
Copy link
Collaborator Author

You are right @rlopezcoto, actually all this can be eliminated with the ctapipe 8.0 charge extractor,
because the corrections are already automatically done at that level (by the way, can one switch the off in ctapipe?).
Do I remove them here?

@rlopezcoto
Copy link
Contributor

Is this at the moment used anywhere? If yes, then you could change the function to that of ctapipe, otherwise you could just removed.
This is not really a change absolutely needed for this step because the code can run anyways without it, so do it only if it is something quick (I would also suggest to separate this in a different PR), otherwise we can wait to the second stage of lstchain adaptation to ctapipe

@rlopezcoto
Copy link
Contributor

@FrancaCassol shall we merge this here and take care of that bug/deletion in a separate PR? I can open an Issue to keep track of it if you cannot do it immediately, but it would be great to have the calibration fixes implemented

@FrancaCassol
Copy link
Collaborator Author

Hi Ruben,
yes accept this, I am now working on new PR where I put all the remaining changes for the having the calibration working (actually there are several others)

@rlopezcoto rlopezcoto merged commit f50911a into cta-observatory:lstchain_ctapipe0.8 Jul 8, 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.

2 participants