-
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
Fix definition of "true disp_sign" parameter #1027
Conversation
WIP, have to change calls to the modified functions. |
lstchain/reco/disp.py
Outdated
# reconstructed image axis (with direction defined by the versor cos(hillas_psi), sin(hillas_psi)) | ||
# we must move from cog_x, cog_y to get closest to the true direction (src_x, src_y) | ||
|
||
sqrdist_plus = (cog_x + disp_norm*np.cos(hillas_psi) - src_x)**2 + (cog_y + disp_norm*np.sin(hillas_psi) - src_y)**2 |
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.
This is a little hard to read...
Is it the same as here?
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.
Looks different and more complicated.
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.
Same result, but ok, the approach you mention is simpler. Will change it.
Simplified calculation of disp. Also, disp_norm slightly changes meaning. Before it was just "dist" (true distance cog to source), now it is distance along reconstructed axis from cog to the point in the axis closest to the source. This is more what we are looking for, so it should be better (difference is likely minimal).
Codecov ReportBase: 71.62% // Head: 71.62% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1027 +/- ##
=======================================
Coverage 71.62% 71.62%
=======================================
Files 120 120
Lines 11301 11302 +1
=======================================
+ Hits 8094 8095 +1
Misses 3207 3207
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
lstchain/reco/disp.py
Outdated
@@ -123,7 +133,7 @@ def disp_parameters_event(hillas_parameters, source_pos_x, source_pos_y): | |||
disp_container.sign = d[4] | |||
disp_container.miss = miss(disp_container.dx.value, |
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.
If the result is multiplied with u.m
, dx and dy must be u.m
, right? So I would add replace value
with to_value(u.m)
, which will save us some headache once we switch to all parameters in telescope frame (degrees) because it will fail instead of silently attach the wrong unit.
No description provided.