-
Notifications
You must be signed in to change notification settings - Fork 157
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: Change theta and eta calls on only 3D Vectors #1706
Conversation
The theta and eta calls could be done on 4D vectors. While they are well defined on a LorentzVector, for example, this was not the intended behaviour of the function, which supposedly should run only on 3D vectors. In particular eta on a 4-momentum was returning the wrong results. Changed the checks on rows to be exactly 3. In the future the eta/theta could be properly defined on 4d vectors as well by choosing a convention for the indices of the 4D vector
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.
LGTM!
Codecov Report
@@ Coverage Diff @@
## main #1706 +/- ##
=======================================
Coverage 49.28% 49.28%
=======================================
Files 398 398
Lines 21787 21787
Branches 9884 9884
=======================================
Hits 10737 10737
Misses 4201 4201
Partials 6849 6849
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
📊 Physics performance monitoring for 79b7e20Full report VertexingSeedingCKFAmbiguity resolutionTruth tracking (Kalman Filter)Truth tracking (GSF) |
Ah you might need to add @pbutti to your CI-bridge-allow-list @paulgessinger ? |
The theta and eta calls could be done on 4D vectors. While they are well defined on a LorentzVector, for example, this was not the intended behaviour of the function, which supposedly should run only on 3D vectors. In particular eta on a 4-momentum was returning the wrong results. Changed the checks on rows to be exactly 3. In the future the eta/theta could be properly defined on 4d vectors as well by choosing a convention for the indices of the 4D vector
The theta and eta calls could be done on 4D vectors. While they are well defined on a LorentzVector, for example, this was not the intended behaviour of the function, which supposedly should run only on 3D vectors. In particular eta on a 4-momentum was returning the wrong results. Changed the checks on rows to be exactly 3. In the future the eta/theta could be properly defined on 4d vectors as well by choosing a convention for the indices of the 4D vector
The theta and eta calls could be done on 4D vectors. While they are well defined on a LorentzVector, for example, this was not the intended behaviour of the function, which supposedly should run only on 3D vectors. In particular eta on a 4-momentum was returning the wrong results. Changed the checks on rows to be exactly 3. In the future the eta/theta could be properly defined on 4d vectors as well by choosing a convention for the indices of the 4D vector