-
Notifications
You must be signed in to change notification settings - Fork 77
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 Tool to to use pyirf IRF interpolation functions #711
Conversation
…rpolation functions
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #711 +/- ##
==========================================
+ Coverage 71.76% 72.77% +1.00%
==========================================
Files 120 122 +2
Lines 11348 11849 +501
==========================================
+ Hits 8144 8623 +479
- Misses 3204 3226 +22
☔ View full report in Codecov by Sentry. |
I have separated the interpolation functions in
Currently, only effective area and energy dispersion IRFs are interpolated and written into a final IRF. The IRF Tool is slightly modified to include necessary metadata values for the interpolation. The DL3 Tool has the following changes:
For testing these functions, I have temporarily created 2 different IRFs, one with different g/h cut and another with an extrapolated values to check interpolation function. This PR still needs further cleaning and more improvements. |
…h dependence, along with some cleaning
…ld and shower axis
… outside a given grid, use the point projected to the closest facet on the grid to use the interpolated value of that point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First quick pass over the code on my side. I will look more into the complex code later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed interpolate.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now went other the vast majority of the code (not always looking into the details).
Once current items are addressed I think the PR will be ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this stage I think the PR is ready for merging.
With the upcoming pyirf PR#150 and the merged PR#141, the DL3 Tool needs to be updated according to Max's suggestion in #696.
As soon as the IO functions in pyirf are merged, I will update the new function
interpolate_irf
inlstchain/irf/hdu_table.py
and with the new MC DL2 productions of different zeniths (same RF model) are ready, the functions can be tested and then I can open the PR.