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

refactor: use grid density seeding #2270

Closed
wants to merge 2 commits into from

Conversation

felix-russo
Copy link
Contributor

Replace gaussian vertex seeding (Sec. 5.4.2 in Bastian Schlag's thesis) with grid vertex seeding (Sec. 5.4.3 in Bastian Schlag's thesis)

@felix-russo felix-russo added 🚧 WIP Work-in-progress Vertexing labels Jul 4, 2023
@felix-russo felix-russo added this to the next milestone Jul 4, 2023
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module labels Jul 4, 2023
@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #2270 (c8f517a) into main (5e1ff46) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2270   +/-   ##
=======================================
  Coverage   49.27%   49.27%           
=======================================
  Files         449      449           
  Lines       25408    25408           
  Branches    11724    11724           
=======================================
  Hits        12521    12521           
  Misses       4554     4554           
  Partials     8333     8333           
Impacted Files Coverage Δ
...include/Acts/Vertexing/GridDensityVertexFinder.hpp 100.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

📊 Physics performance monitoring for c8f517a

Summary
Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
AMVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu
IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

AMVF seeded

AMVF truth_smeared

AMVF truth_estimated

AMVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

@@ -15,6 +15,8 @@
#include "Acts/Vertexing/Vertex.hpp"
#include "Acts/Vertexing/VertexingOptions.hpp"

#include <map>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this addition?

@@ -22,7 +22,8 @@
#include "Acts/Vertexing/HelicalTrackLinearizer.hpp"
#include "Acts/Vertexing/ImpactPointEstimator.hpp"
#include "Acts/Vertexing/IterativeVertexFinder.hpp"
#include "Acts/Vertexing/TrackDensityVertexFinder.hpp"
//#include "Acts/Vertexing/TrackDensityVertexFinder.hpp"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove commented pieces of code

@CarloVarni
Copy link
Collaborator

Hi @felix-russo I see there is the WIP label. Is this ready for review, or is it a draft PR?

@felix-russo
Copy link
Contributor Author

Hi @felix-russo I see there is the WIP label. Is this ready for review, or is it a draft PR?

Hi @CarloVarni, no, this PR is not ready for review yet! I just wanted to know how the different seed finder affects the physics performance... I will look into this further and let you know when I am done!

@felix-russo felix-russo marked this pull request as draft July 7, 2023 10:05
@LuisFelipeCoelho
Copy link
Member

Hi @felix-russo I see there is the WIP label. Is this ready for review, or is it a draft PR?

Hi @CarloVarni, no, this PR is not ready for review yet! I just wanted to know how the different seed finder affects the physics performance... I will look into this further and let you know when I am done!

just to mention that you can also run physmon locally to check the performance, if you prefer

@felix-russo
Copy link
Contributor Author

Will investigate this later

@paulgessinger paulgessinger modified the milestones: next, v27.2.0 Jul 24, 2023
@felix-russo felix-russo deleted the grid-seeding branch August 30, 2023 12:42
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 Component - Examples Affects the Examples module 🚧 WIP Work-in-progress Vertexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants