Properly transform between camera and telescope frame in muon fitter #2464
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2464 +/- ##
==========================================
- Coverage 92.47% 92.46% -0.01%
==========================================
Files 234 234
Lines 19816 19811 -5
==========================================
- Hits 18325 18319 -6
- Misses 1491 1492 +1 ☔ View full report in Codecov by Sentry. |
maxnoe
force-pushed
the
fix_muon_test
branch
from
437c75a
to
011d6d7
Compare
There was a problem hiding this comment.
Given that this fixes #2467 and with that the effective vs. equivalent focal-length discrepancy with, according to the issue, up to 25% differences in the muon efficiencies, I'd argue that this needs a changelog entry.
|
@RuneDominik Yes, good point! I oversaw this change before. |
maxnoe
force-pushed
the
fix_muon_test
branch
from
2d4c115
to
edbdeb9
Compare
maxnoe
force-pushed
the
fix_muon_test
branch
from
edbdeb9
to
31e1c1e
Compare
| @@ -309,7 +308,8 @@ def image_prediction_no_units( | |||
|
|
|||
| def build_negative_log_likelihood( | |||
| image, | |||
| telescope_description, | |||
| optics, | |||
| geometry_tel_frame, | |||
There was a problem hiding this comment.
So here you change the function signature, but I can't see any test being modified to match this new signature. Did I simply miss it or is there intentionally no test of build_negative_log_likelihood?
There was a problem hiding this comment.
The test is through the __call__ method of the class, there is no direct test of this method.
The muon code did some strange things to transform pixel coordinates from camera to telescope frame, probably from code before camera geometries could be transformed as a whole.