Update DL3 Tools with energy dependent optimized cuts #709
Conversation
Codecov Report
@@ Coverage Diff @@
## master #709 +/- ##
==========================================
+ Coverage 84.65% 84.92% +0.27%
==========================================
Files 77 77
Lines 6074 6170 +96
==========================================
+ Hits 5142 5240 +98
+ Misses 932 930 -2
Continue to review full report at Codecov.
|
|
Sorry for the long delay on this PR. The work on energy-dependent cuts for gammaness is ready. However, for cuts on the theta parameter, it is not clear how we should define the ON/OFF region. There is an ongoing work in gammapy (gammapy/gammapy#3684) on energy-dependent 1D spectrum extraction, and once it is resolved/merged there, a separate PR can be made in lstchain to appropriately prepare the DL3 files to have the energy-dependent theta cuts and store the data event lists appropriately. If there is no other issue on this, can this PR be merged before the Analysis School begins next week, or for the 0.8.3 release? |
|
Hi @chaimain this is a bit long PR that has not been looked at yet, is this absolutely necessary for the school? Otherwise it may be a better idea to leave its merging for afterwards |
The ongoing work is on how to implement the analysis for the already defined format. The required format will not change as a result of that work. You just need to include the |
|
@rlopezcoto It is not absolutely essential, but if it were to be included, it would be great. It is my fault for delaying this one for so long. The school can work fine without this PR. |
True, but I am more concerned about how we use separate ON/OFF event lists, based on the theta cuts we apply. It can be kept up to the user on how they work it through maybe, but I would like it to be at least indicated in the Tool, or shown in an example notebook on how it can be done. So, I wanted to keep that work separate from this PR |
We don't use separated ON/OFF event lists at the DL3 level. DL3 files should include all events surviving gammaness cuts without applying a directional cut with respect to any on or off region. The work in gammapy is precisely on how off regions are determined and how the theta cuts are applied. This is the domain of gammapy, not the DL3 export. |
So, are you implying that the energy-dependent directional cuts should be applied to all the events in MC and observed data? |
I don't understand the question. The cuts need to be applied on simulated events for calculating the IRFs, but they must not be already applied for simulated test datasets or observed data, as again, this is done later in gammapy and the DL3 events list must not have a directional cut already applied, as this prevents letting gammapy or any other science tool chose the source to be analyzed and the off regions. There are two parts to the directional cut: the (possibly energy dependent) radius and the position around which this radius is applied. The first thing needs to be fixed by us and written into the file (the Again, no direction cut applied on events in DL3 event lists. |
|
Ok, I understand this now. Thank you for clarifying this. I will amend the PR accordingly. |
…atory/cta-lstchain into energy_dependent_cuts
| --energy-dependent-gh | ||
| --energy-dependent-theta |
There was a problem hiding this comment.
Is there an advantage in having these? It would be enough to activate the E-dependent cuts if the --gh-efficiency and/or --theta-containment options below are used, right?
There was a problem hiding this comment.
Yes, but this would need to change the defaults of the configuration to have them all None and activate the corresponding setting by giving it.
And I think there is a bug in traitlets that doesn't allow a Float(allow_none=True):
I fixed it for ctapipe_io_lst here:
There was a problem hiding this comment.
The config file may have the options, but it will be up to the user to pass these separate flags to use the energy-dependent cuts
There was a problem hiding this comment.
Yes, we understand the current situation. We are proposing to change that.
The issue seems to be fixed in traitlets 5.1 we are using now.
…de in the HDU Index files
…the gh_efficiency and theta_containment to be provided in % without changing the values in the Class Merge branch 'master' of https://github.com/cta-observatory/cta-lstchain into energy_dependent_cuts
…RF, fix the pytest of creating RAD_MAX with diffuse gamma across multiple FoV bins
Adding an option (for now), to use optimised (for now only g/h separation) cuts for MC and observed data, for the DL3 files.
Optimization is done on a fixed gamma efficiency for each reco energy bin, using pyirf functions.
I have to test this a bit more before making it open PR.