acts-project
feat: Hashing seeding algorithm
Adds the Hashing for the seeding algorithm.
Instead of doing the seeding on every space points at once, this approach create small groups of space points called buckets and do the standard seeding on each of those buckets independently.
As there is overlaps between the buckets, the same seed might be reconstructed several times (due to independent reconstruction in several buckets). A set container of seeds is used to naturally handle this duplication, hence the need of #3136 and sorting the seeds ad19e7a9472b980decc89a052f358a07fe617400.
This approach mitigate the filtering of good seeds due to an upper limit on the number of seeds sharing the same middle space point (the maxSeedsPerSpM parameter).
More details are in the poster presented on CTD 2023.
A third party software called Annoy is used to create the buckets. Some modifications of the software code has been done with respect to the official repository linked previously.
Blocked by:
- #3136
📊: Physics performance monitoring for 3eafd394d3fe7c34b78599f9add31881deec3ed9
physmon summary
- ✅ Particles fatras
- ✅ Particles geant4
- ✅ Particles ttbar
- ✅ Vertices ttbar
- ✅ Truth tracking (KF)
- ✅ Truth tracking (GSF)
- ✅ Truth tracking (GX2F)
- ✅ CKF | trackfinding | single muon | truth smeared seeding
- ✅ Track Summary CKF | trackfinding | single muon | truth smeared seeding
- ✅ Seeding trackfinding | single muon | truth estimated seeding
- ✅ CKF | trackfinding | single muon | truth estimated seeding
- ✅ Track Summary CKF | trackfinding | single muon | truth estimated seeding
- ✅ Seeding trackfinding | single muon | default seeding
- ✅ CKF | trackfinding | single muon | default seeding
- ✅ Track Summary CKF | trackfinding | single muon | default seeding
- ✅ Seeding trackfinding | single muon | orthogonal seeding
- ✅ CKF | trackfinding | single muon | orthogonal seeding
- ✅ Track Summary CKF | trackfinding | single muon | orthogonal seeding
- ✅ Seeding trackfinding | 4 muon x 50 vertices | default seeding
- ✅ CKF | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ Track Summary CKF | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ Ambisolver | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ IVF notime | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ AMVF gauss notime | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ AMVF grid time | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ Seeding trackfinding | ttbar with 200 pileup | default seeding
- ✅ CKF | trackfinding | ttbar with 200 pileup | default seeding
- ✅ Track Summary CKF | trackfinding | ttbar with 200 pileup | default seeding
- ✅ Ambisolver | trackfinding | ttbar with 200 pileup | default seeding
- ✅ AMVF gauss notime | trackfinding | ttbar with 200 pileup | default seeding
- ✅ AMVF grid time | trackfinding | ttbar with 200 pileup | default seeding
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Jeremy pinged me privately on it, but I thought it would make more sense to answer here, publicly.
What you forgot about updating / extending is this file: https://github.com/acts-project/acts/blob/main/cmake/ActsConfig.cmake.in
You are now introducing a new external, which ActsCore would now publicly depend on. (Which in itself is a bit worrisome to me, but that's a separate issue...) So you must make sure that when somebody calls find_package(Acts), ActsConfig.cmake would call find_dependency(Annoy) itself.
Since in this PR's setup Annoy would always be needed, you should add
find_dependency(Annoy)
without any if(...) statements around here: https://github.com/acts-project/acts/blob/main/cmake/ActsConfig.cmake.in#L55
Actually, the logic of only calling find_dependency(Boost) and find_dependency(Eigen3) is not great over there. Since even if Acts was the one building / installing those, the Acts installation should still include everything required to make those find_dependency(...) calls work. :thinking: (So they should always be called.)
But that's a separate issue. For this PR, just add find_dependency(Annoy ...) with all the right arguments.
Codecov Report
Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
Project coverage is 47.43%. Comparing base (
75ef913) to head (857e6f1). Report is 3 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| Core/include/Acts/Seeding/ContainerPolicy.hpp | 0.00% | 6 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #3148 +/- ##
==========================================
- Coverage 47.66% 47.43% -0.23%
==========================================
Files 509 511 +2
Lines 29425 30053 +628
Branches 14131 14561 +430
==========================================
+ Hits 14026 14257 +231
- Misses 5285 5326 +41
- Partials 10114 10470 +356
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hi @CouthuresJeremy we have merged https://github.com/acts-project/acts/pull/336 . Can you adapt this PR to those changes?
Hi, I am on vacation without any computer to update this PR until the 29th of July. I haven't been able to test #3367 yet but thanks for making it!
Two considerations:
1. I'm not sure it's the greatest solution to ship Annoy in our `thirdparty`. We don't necessarily do this for all dependencies (like root and g4). Would it be an alternative (maybe in a follow up PR) to push this to our _externals_ build and use `find_package` for this?
I do not really see the cons of having Annoy in the externals build and use find_package to get it instead of having it in thirdparty. Would this have consequences on updating the patch?
2. It's usually a good idea to limit exposure to third party headers. I'm wondering if the annoy includes could potentially be pushed to private includes in cpp files.
Do you mean in the SeedingAlgorithmHashing example or in the plugin?
Overall, in this approach, only a clustering algorithm is needed, not Annoy specifically. So it could be replaced latter if needed.
Quality Gate passed
Issues
23 New issues
0 Accepted issues
Measures
0 Security Hotspots
66.8% Coverage on New Code
11.1% Duplication on New Code
See analysis details on SonarCloud
![sonarqubecloud[bot] avatar](https://avatars.githubusercontent.com/in/12526?v=4 "Published by a pub.dev verified publisher"){kind=small}
:red_circle: Athena integration test results [2e0a7d13bac292112a1e339178edf145500c1abc]
:red_circle: Some tests have failed!
Please investigate the pipeline!
