-
Notifications
You must be signed in to change notification settings - Fork 173
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
docs: Add comment on computation of impact parameter from the doublet #3728
Merged
kodiakhq
merged 7 commits into
acts-project:main
from
CarloVarni:CorrectImpactComputation
Oct 16, 2024
Merged
docs: Add comment on computation of impact parameter from the doublet #3728
kodiakhq
merged 7 commits into
acts-project:main
from
CarloVarni:CorrectImpactComputation
Oct 16, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CarloVarni
changed the title
fix: Computation of impact parameter from the doublet
doc: Add comment on computation of impact parameter from the doublet
Oct 15, 2024
/cc @noemina |
CarloVarni
changed the title
doc: Add comment on computation of impact parameter from the doublet
docs: Add comment on computation of impact parameter from the doublet
Oct 15, 2024
andiwand
approved these changes
Oct 16, 2024
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 👍
Quality Gate passedIssues Measures |
acts-project-service
added
the
Breaks Athena build
This PR breaks the Athena build
label
Oct 16, 2024
Rosie-Hasan
pushed a commit
to Rosie-Hasan/acts
that referenced
this pull request
Nov 13, 2024
…acts-project#3728) One of the criterion used in doublet finding relies on computing the minimal distance between the origin and the straight line connecting the two points of the doublet. This distance is compared against a maximum impact parameter (`impactMax`) provided by the user. Let's consider for instance the middle-top doublet search. <img width="479" alt="Geometry" src="https://github.com/user-attachments/assets/bb1c3f87-d687-4f49-9582-02e91773f8ad"> In the attached image: - `A` is the origin - `B` is the middle space point, thus `AB = rM` (radius of middle space point) - `C` is the top space point What the code does is to compute `BD` (`xNewFrame` in the code) and `CD` (`yNewFrame` in the code) and then a simple geometric similarity to compute the impact parameter. The code currently does: `Im / CD = AB / BD`, where `Im` is the impact parameter we are trying to compute. Thus `Im = (AB * CD) / BD <= impactMax`. Unfortunately `Im` is equal to `AF`, while we really want to compute `AE`. The proper computation would be `Im / CD = AB / BC`, which leads to `Im = (AB * CD) / BC <= impactMax`. In this case `Im = AE`. However, `BC` has to be computed from `CD` and `BD`. In order to avoid square roots I'm using square quantities. It is also possible that the code assumes that the `alpha` angle is too small to make a difference and thus uses an approximation... But there are no comments in the code about this
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Breaks Athena build
This PR breaks the Athena build
Component - Core
Affects the Core module
Seeding
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of the criterion used in doublet finding relies on computing the minimal distance between the origin and the straight line connecting the two points of the doublet. This distance is compared against a maximum impact parameter (
impactMax
) provided by the user.Let's consider for instance the middle-top doublet search.
In the attached image:
A
is the originB
is the middle space point, thusAB = rM
(radius of middle space point)C
is the top space pointWhat the code does is to compute
BD
(xNewFrame
in the code) andCD
(yNewFrame
in the code) and then a simple geometric similarity to compute the impact parameter.The code currently does:
Im / CD = AB / BD
, whereIm
is the impact parameter we are trying to compute. ThusIm = (AB * CD) / BD <= impactMax
. UnfortunatelyIm
is equal toAF
, while we really want to computeAE
. The proper computation would beIm / CD = AB / BC
, which leads toIm = (AB * CD) / BC <= impactMax
. In this caseIm = AE
.However,
BC
has to be computed fromCD
andBD
. In order to avoid square roots I'm using square quantities.It is also possible that the code assumes that the
alpha
angle is too small to make a difference and thus uses an approximation... But there are no comments in the code about this