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

Update DL3 cuts and some clean-up #1105

Merged
merged 13 commits into from
Jun 7, 2023

Conversation

chaimain
Copy link
Contributor

@chaimain chaimain commented May 18, 2023

Instead of a max_theta_cut for energy-dependent theta (source-independent) cuts, RAD_MAX, evaluate the maximum RAD_MAX as per the wobble offset angle and the number of OFF regions to be used in ON/OFF normalization, in post DL3 analysis, when using Wobble OFF regions to evaluate the background counts. This replaces max_theta_cut and fill_theta_cut with n_off_wobbles in this case. Evaluating the maximum energy-dependent theta cuts should be analysis dependent and therefore up to the analyzer to change the values in the config and not let the code arrange it using a fixed method (no standard has been defined).

  • Add a function update_fill_cuts to replace the cut values for the energy-dependent cuts evaluated with pyirf, for the energy bins with fewer events than min_event_p_en_bin (default = 100), with the cut value of the nearest energy bin, evaluated properly. This is usually relevant for lower and higher energy bins, where there are fewer or no MC events, and will select a cut evaluated for the first or last energy bin with a statistically significant number of events, instead of a constant fill_value. This is applied for all such parameters - gammaness, theta and alpha (source-dependent).
  • Some general clean-up of code in lstchain/high_level/hdu_table and updated docstrings.

Note: These changes will make the IRF/DL3 files incompatible with older versions

chaimain added 2 commits May 18, 2023 14:52
…o update en-dep cuts for bins with fewer events than minimum, with nearest evaluated cut values
@chaimain chaimain requested review from SeiyaNozaki and moralejo May 18, 2023 19:11
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.06 🎉

Comparison is base (aa0af55) 74.57% compared to head (3d160c3) 74.64%.

Additional details and impacted files
@@               Coverage Diff                @@
##           ctapipe_0.17    #1105      +/-   ##
================================================
+ Coverage         74.57%   74.64%   +0.06%     
================================================
  Files               124      124              
  Lines             12321    12352      +31     
================================================
+ Hits               9189     9220      +31     
  Misses             3132     3132              
Impacted Files Coverage Δ
lstchain/high_level/__init__.py 100.00% <ø> (ø)
lstchain/high_level/tests/test_interpolate.py 100.00% <ø> (ø)
lstchain/io/tests/test_io.py 100.00% <ø> (ø)
lstchain/reco/utils.py 76.38% <ø> (ø)
lstchain/tools/tests/test_tools.py 100.00% <ø> (ø)
lstchain/high_level/hdu_table.py 95.00% <100.00%> (-0.05%) ⬇️
lstchain/high_level/tests/test_hdus.py 100.00% <100.00%> (ø)
lstchain/io/event_selection.py 100.00% <100.00%> (ø)
lstchain/io/tests/test_eventselection.py 100.00% <100.00%> (ø)
lstchain/tools/lstchain_create_dl3_file.py 83.83% <100.00%> (-0.10%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chaimain
Copy link
Contributor Author

chaimain commented May 25, 2023

Testing with an older MC production using the current master dependencies, and using the new (pyirf v0.8) n_events Column data for the cuts table, to be consistent with the functionality of update_fill_cuts, to check for energy-dependent gammaness and theta cuts.
fill_dl3_cuts

For testing with the updated dependencies, we would need a new MC DL2 (at least) production, due to the new methods of ctapipe in reading the simulation info (eg num_showers -> n_showers)

@moralejo
Copy link
Collaborator

fill_dl3_cuts

It is a bit weird though that the gammaness cut decreases after 8 TeV. Hard to be sure on a color scale, but it does not seem like the gammaness distributions shift down as energy increases... What efficiency is that? Do the cuts seem right (e.g. for the first bin above 10 TeV) if the vertical slices of the histogram are plotted?

@chaimain
Copy link
Contributor Author

fill_dl3_cuts

It is a bit weird though that the gammaness cut decreases after 8 TeV. Hard to be sure on a color scale, but it does not seem like the gammaness distributions shift down as energy increases... What efficiency is that? Do the cuts seem right (e.g. for the first bin above 10 TeV) if the vertical slices of the histogram are plotted?

The cut efficiency is default value of 0.95. In a coarser binning (10 instead of earlier 40) for gammaness and with plotting bins with minimum 30 events, we do see a collection of events with lower gammaness value.
fill_dl3_cuts_2

The majority of events do have gammaness value close to the maximum, but because of these group of events, the percentile evaluation yields a lower cut value. The cuts of 95% efficiency holds true for these bins.

Copy link
Collaborator

@SeiyaNozaki SeiyaNozaki left a comment

Choose a reason for hiding this comment

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

The computed alpha cuts look as expected, so this PR looks fine for me (source-dep analysis). I just added a small comment.

lstchain/io/event_selection.py Show resolved Hide resolved
@rlopezcoto
Copy link
Contributor

@moralejo changes were made, anything else missing?

@rlopezcoto rlopezcoto merged commit f859c74 into ctapipe_0.17 Jun 7, 2023
@rlopezcoto rlopezcoto deleted the ctapipe_0.17_update_dl3_cuts branch June 7, 2023 16:00
@chaimain chaimain restored the ctapipe_0.17_update_dl3_cuts branch June 7, 2023 17:37
@chaimain chaimain mentioned this pull request Jun 8, 2023
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.

5 participants