Allow using custom energy dependent cuts (e.g. based on sensitivity optimization) for IRFs and DL3 #1348
Conversation
… in the irf and dl3 creation tools.
|
Hi @gabemery, thanks for the implementation, I think it makes sense to support this. Maybe have a look at how it was recently done in ctapipe, so here we use similar naming and so on (cta-observatory/ctapipe#2473). Specifically, they implement: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1348 +/- ##
==========================================
+ Coverage 72.86% 72.99% +0.12%
==========================================
Files 137 137
Lines 14515 14583 +68
==========================================
+ Hits 10577 10645 +68
Misses 3938 3938 ☔ View full report in Codecov by Sentry. |
|
@morcuended Thanks for bringing the ctapipe version to my attention. I will have a look and see what can be implemented without re-writing the full tool. |
|
After having a look, there is nothing to directly implement from the ctapipe irf production into this PR since the pipelines are very different. |
|
@morcuended can you approve? I had a quick look and it seems ok to me, but probably you know this better. |
|
Let me have a look first |
morcuended
changed the title
| help="True for applying energy-dependent gammaness cuts", | ||
| default_value=False, | ||
| cut_strategy_gh = traits.Unicode( | ||
| help="Define the strategy used for the gammaness cut." |
There was a problem hiding this comment.
| help="Define the strategy used for the gammaness cut." | |
| help="Strategy used for the gammaness cut." |
| help="True for applying energy-dependent theta cuts", | ||
| default_value=False, | ||
| cut_strategy_theta = traits.Unicode( | ||
| help="Define the strategy used for the theta cut." |
There was a problem hiding this comment.
| help="Define the strategy used for the theta cut." | |
| help="Strategy used for the theta cut." |
| help="True for applying energy-dependent alpha cuts", | ||
| default_value=False, | ||
| cut_strategy_alpha = traits.Unicode( | ||
| help="Define the strategy used for the alpha cut." |
There was a problem hiding this comment.
| help="Define the strategy used for the alpha cut." | |
| help="Strategy used for the alpha cut." |
| interpolate_custom = traits.Unicode( | ||
| help="Interpolation option for scipy.interpolate.interp1d used with " | ||
| "custom, energy dependent cuts.", | ||
| default_value="global", |
There was a problem hiding this comment.
default "linear", "nearest"? not global
| default_value="global", | ||
| ).tag(config=True) | ||
|
|
||
| interpolate_custom = traits.Unicode( |
There was a problem hiding this comment.
I would name this argument differently to indicate that you're interpolating cut values. Something like interp_method_custom_cuts
|
|
||
| input_ebins_center = (cuts_dict["energy_bins"][:-1]* u.TeV + cuts_dict["energy_bins"][1:] * u.TeV) * 0.5 | ||
|
|
||
| cut_table["low"] =energy_bins[:-1] |
There was a problem hiding this comment.
| cut_table["low"] =energy_bins[:-1] | |
| cut_table["low"] = energy_bins[:-1] |
| 'gh', 'theta' or 'alpha' | ||
| energy_bins: `astropy.units.quantity.Quantity` | ||
| energy bins used for the IRFs | ||
| interpolate_kind: string |
There was a problem hiding this comment.
I'd use interpolation instead of interpolate here
| def from_dict(self, parameter, energy_bins, interpolate_kind='nearest'): | ||
| """ | ||
| Convert the cut dictionary to a QTable for the requested parameter. | ||
|
|
There was a problem hiding this comment.
I'd also add that the cut values are interpolated along the IRF energy binning.
|
|
||
| Returns | ||
| ------- | ||
| cut_table: QTable |
There was a problem hiding this comment.
| cut_table: QTable | |
| cut_table: `astropy.table.QTable` |
| ---------- | ||
| parameter: string | ||
| 'gh', 'theta' or 'alpha' | ||
| energy_bins: `astropy.units.quantity.Quantity` |
There was a problem hiding this comment.
maybe explicitly irf_energy_bins?
Currently, IRF and DL3 creation tools only allow for global or energy-dependent gammaness, theta or alpha cuts calculated based on MC gamma-ray efficiency.
These changes introduce the option to provide custom previously calculated energy-dependent cuts on gammaness, theta or alpha during the IRFs creation. And provides associated metadata in the IRFs and DL3 files.
Why is it needed : Allows to provide energy dependent cuts based on criteria different from gamma efficiency. Such as best sensitivity.
Tool usage change with the new implementation :
Removed flags --energy-dependent-gh, --energy-dependent-theta and --energy-dependent-alpha and replaced them by a --cut_strategy-{gh, theta, alpha} which receives the technique used as a string input.
The implementation is near complete and can be reviewed, but some points need discussion/further work.
To do before merging :