Skip to content
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

Use HierarchicalGeometryContainer to simplfiy CKF source link selection #168

Merged

Conversation

XiaocongAi
Copy link
Contributor

This PR simplify the CKF source link selection by using the hierarchical geometry container for the selection criteria.

@acts-issue-bot acts-issue-bot bot added the Triage label May 1, 2020
@XiaocongAi XiaocongAi added the Improvement Changes to an existing feature label May 1, 2020
@acts-issue-bot acts-issue-bot bot removed the Triage label May 1, 2020
@XiaocongAi XiaocongAi changed the title Use hierarchical geometry container for CKF source link selection Use HierarchicalGeometryContainer to simplfiy CKF source link selection May 1, 2020
@codecov
Copy link

codecov bot commented May 1, 2020

Codecov Report

Merging #168 into master will increase coverage by 0.02%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #168      +/-   ##
==========================================
+ Coverage   46.01%   46.04%   +0.02%     
==========================================
  Files         353      353              
  Lines       17664    17646      -18     
  Branches     8220     8209      -11     
==========================================
- Hits         8128     8125       -3     
+ Misses       4319     4312       -7     
+ Partials     5217     5209       -8     
Impacted Files Coverage Δ
...include/Acts/TrackFinder/CKFSourceLinkSelector.hpp 47.82% <57.14%> (+6.25%) ⬆️
...de/Acts/Geometry/HierarchicalGeometryContainer.hpp 55.00% <0.00%> (-0.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f540b6a...4bc59df. Read the comment docs.

@XiaocongAi XiaocongAi added Component - Core Affects the Core module Impact - Minor Nuissance bug and/or affects only a single module labels May 2, 2020
@XiaocongAi
Copy link
Contributor Author

Hi @msmk0 , could you help take a look at this PR to see if the HierarchicalGeometryContainer is used in the right way?

@XiaocongAi XiaocongAi force-pushed the simplify_source_link_selection branch from bcbfb59 to aa040c8 Compare May 4, 2020 05:01
@XiaocongAi XiaocongAi force-pushed the simplify_source_link_selection branch from aa040c8 to 285ad9a Compare May 5, 2020 22:39
@XiaocongAi
Copy link
Contributor Author

XiaocongAi commented May 5, 2020

@msmk0, I have combined the container for chi2 and nSourcelinks configuration. Could we merge it in and handle the others in issue #178 ?

@msmk0 msmk0 self-requested a review May 6, 2020 07:31
@msmk0
Copy link
Contributor

msmk0 commented May 6, 2020

I approved the changes and the CI build runs through, but I can not merge. It looks like some checks are blocking it. Not sure what to do in that case. @paulgessinger Suggestions?

@acts-issue-bot acts-issue-bot bot removed the Triage label May 6, 2020
@msmk0 msmk0 merged commit c74a7e7 into acts-project:master May 6, 2020
@paulgessinger
Copy link
Member

Seems to have been a communication problem with the bots.

@paulgessinger paulgessinger added this to the v0.24.00 milestone May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Impact - Minor Nuissance bug and/or affects only a single module Improvement Changes to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants