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

Fix muon scripts #232

Merged
merged 7 commits into from
Nov 26, 2019
Merged

Conversation

rlopezcoto
Copy link
Contributor

No description provided.

Copy link
Member

@morcuended morcuended left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a quick look and seems fine to me. Just x-check how you are using the image array in the lstchain_data_muon_analysis.py

@@ -153,17 +153,17 @@ def main():
print("Event {}. Number of pixels above 10 phe: {}".format(event_id,
np.size(image[0][image[0] > 10.])))

if not tag_pix_thr(image): #default skipps pedestal and calibration events
if not tag_pix_thr(image[0]): #default skipps pedestal and calibration events
Copy link
Member

@morcuended morcuended Nov 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change this back to image[0] while in the rest of the script you corrected to just image?. In this script, does the image array still contain the two gains for each pixel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right @morcuended. I should apply here gain selection...
by the way, for the data that you are analyzing in LP, are you applying the proper threshold for the gain selection? According to #200, we do not have a proper threshold due to the ctapipe bug

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have the threshold set to 4094, then it means that the threshold is not properly applied.

For the muon script, I do not think it is a good idea to have a duplicated calibration for the regular data analysis and muon data analysis, therefore I expect the script lstchain_data_muon_analysis.py to be deprecated and run the analysis only on calibrated files. I'll mark this in a comment on the script as no-gain selection, but I'm not sure if it is worth it to solve it at the moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the warning I'm getting:

WARNING: AstropyDeprecationWarning: channel selection is now performed at <= R1 calibration level [lstchain.reco.dl0_to_dl1]
WARNING: AstropyDeprecationWarning: gain selection is now performed at <= R1 calibration level [lstchain.calib.camera.calib]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have the threshold set to 4094, then it means that the threshold is not properly applied.

For the muon script, I do not think it is a good idea to have a duplicated calibration for the regular data analysis and muon data analysis, therefore I expect the script lstchain_data_muon_analysis.py to be deprecated and run the analysis only on calibrated files. I'll mark this in a comment on the script as no-gain selection, but I'm not sure if it is worth it to solve it at the moment.

The config files I am using indicate:

"gain_selector": "ThresholdGainSelector",
 "gain_selector_config": {
   "threshold":  4094
 }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what threshold value should we use then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then I think it will be affected... According to #200, the threshold should be 20.9
@vuillaut can you confirm?

Copy link
Member

@vuillaut vuillaut Nov 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For MC yes.

@moralejo
Copy link
Collaborator

moralejo commented Nov 26, 2019 via email

@morcuended
Copy link
Member

The exact switching value is not so critical, around 20 should be ok, enough to have already a clear LG signal, but could also be taken up to 50 or so safely, I think. More important: the DL1 you were producing then had effectively only high gain, correct?

Yes, I think so. I did not realize this before. Thus, I should set the threshold to 20.9 and reprocess everything again, right?

Copy link
Member

@morcuended morcuended left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead with these scripts

@rlopezcoto
Copy link
Contributor Author

Thanks!

@rlopezcoto rlopezcoto merged commit 6412af1 into cta-observatory:master Nov 26, 2019
@rlopezcoto rlopezcoto deleted the fix_muon_scripts branch April 9, 2020 11:06
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.

4 participants