-
Notifications
You must be signed in to change notification settings - Fork 6
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
ENH: support multiple scattering planes for KF #78
Conversation
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.
One question I have
testbeam_analysis/track_analysis.py
Outdated
z_scatter: z position of scattering plane in um | ||
material_budget_scatter: material budget of scattering plane | ||
alignment_scatter: list which contains alpha, beta and gamma angles of scattering plane. | ||
If None, no rotation will be considered. | ||
In case of multiple scattering planes, each value of a key is a list, with items corresponding to each scatterong plane. |
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.
Typo: scatterong
testbeam_analysis/track_analysis.py
Outdated
z_scatter: z position of scattering plane in um | ||
material_budget_scatter: material budget of scattering plane | ||
alignment_scatter: list which contains alpha, beta and gamma angles of scattering plane. | ||
If None, no rotation will be considered. | ||
In case of multiple scattering planes, each value of a key is a list, with items corresponding to each scatterong plane. | ||
If whole dict is False, no scattering plane will be added. |
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 add_scattering_plane is false
dut_fit_selection[np.where(dut_fit_selection > (index_scatter - 1))[0][0]:] = dut_fit_selection[np.where(dut_fit_selection > (index_scatter - 1))[0][0]:] + 1 | ||
if add_scattering_plane: | ||
for i in range(len(index_scatter)): # need to shift dut fit selection in case of additional scattering plane | ||
dut_fit_selection[np.where(dut_fit_selection > (index_scatter[i] - 1))[0][0]:] = dut_fit_selection[np.where(dut_fit_selection > (index_scatter[i] - 1))[0][0]:] + 1 |
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.
Since a scattering plane is no DUT I wonder why dut_fit_selection
is changed.
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 has also to be changed, since the dut_index is different if additional scattering planes are considered. E.g. if we want to use DUT 0 and 1 in fit, and add a scattering plane in between, the dut_indices are now: 0, 1 (scattering plane) and 2. Then dut_fit_selection
has to be changed to dut_fit_selection = [0, 2]
.
Looks good to me! |
This pull request is related to requested changes for additional scattering plane option for KF
ENH:
MAINT:
change default case from
None
toFalse
remove redundant
index_scatter