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: Vertex performance monitoring #1417

Merged
merged 58 commits into from
Sep 6, 2022

Conversation

paulgessinger
Copy link
Member

Previously, the vertex performance writer would assume that there's a 1:1 correspondence between fitted tracks and truth particles. This prevented to run it on track finding output, where we have many more tracks, esp. without ambiguity resolution.

This PR implements hit based truth matching between tracks and particles and uses that to define truth vertices, etc.
I have to change the minimum track fraction to consider a vertex truth matched drastically, which I'm guessing is because of the aforementioned high duplicate count: if you have a ton of tacks, even correctly reconstructed tracks will likely have a bunch of fake tracks attached.

I also switched the physmon particle gun to 201 vertices with beamspot smearing (but the vertexing still runs without beamspot constraint). The IVF gives something like 50 vertices per event, while the AMVF only finds a handful.

Finally, I added a script to make histograms from the performance ntuple and now feed that into the physmon histogram comparison.

@paulgessinger paulgessinger added this to the next milestone Aug 11, 2022
@paulgessinger
Copy link
Member Author

While the CI histograms are still running, here are some histograms i made with HardQCD + SoftQCD in pythia:

covXX
covYX
covYY
diffx
diffz
nRecoVtx
nTrueVtx
recoOverTrue

Seems consistent to me with the observation that the efficiency with the default configuration at this time isn't great (#1362)

@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Merging #1417 (4d51380) into main (5f4052a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1417   +/-   ##
=======================================
  Coverage   48.59%   48.59%           
=======================================
  Files         381      381           
  Lines       20631    20631           
  Branches     9463     9463           
=======================================
  Hits        10026    10026           
  Misses       4066     4066           
  Partials     6539     6539           

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

@paulgessinger paulgessinger added the 🚧 WIP Work-in-progress label Aug 11, 2022
@paulgessinger
Copy link
Member Author

Ah right, this currently also includes some extra stuff that I'll rip out and put into separate PRs.

@paulgessinger
Copy link
Member Author

I slightly modified the ODD full chain example, to get some numbers as a function of pileup:

image
image

@paulgessinger
Copy link
Member Author

Looking at the code a bit more, it seems like the AMVF in this scenario loses most of the vertex candidates because not tracks are compatible with the vertex after being added to the fit. This could be in fact a result of the high duplication rate that @andiwand was observing as a consequence of fakes influencing the vertex fit.

@paulgessinger
Copy link
Member Author

Hm, I think the perfmon job takes a lot longer than before. I'm guessing this has to do with the increased pseudo-pileup count. I'll see if I can tune this to complete in a reasonable time while giving enough stats.

@paulgessinger paulgessinger removed the 🚧 WIP Work-in-progress label Sep 2, 2022
@paulgessinger
Copy link
Member Author

I think this is finally good to merge, @asalzburger @andiwand.

Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

looks good 👍

CI/physmon/phys_perf_mon.sh Outdated Show resolved Hide resolved
# Conflicts:
#	CI/physmon/reference/acts_analysis_residuals_and_pulls.root
#	CI/physmon/reference/performance_ckf_tracks_seeded.root
#	CI/physmon/reference/performance_ckf_tracks_truth_estimated.root
#	CI/physmon/reference/performance_ckf_tracks_truth_smeared.root
@paulgessinger
Copy link
Member Author

Ok references updates after rebase, can you approve again @andiwand ?

@kodiakhq kodiakhq bot merged commit f5d312f into acts-project:main Sep 6, 2022
@github-actions github-actions bot removed the automerge label Sep 6, 2022
@paulgessinger paulgessinger deleted the vtx-perf branch September 19, 2022 08:45
@paulgessinger paulgessinger added the backport develop/v19.x Backport this PR to the v19.x series label Sep 19, 2022
@acts-project-service
Copy link
Collaborator

The backport to develop/v19.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-develop/v19.x develop/v19.x
# Navigate to the new working tree
cd .worktrees/backport-develop/v19.x
# Create a new branch
git switch --create backport-1417-to-develop/v19.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f5d312f98ad92f6b94d82bfac6108d38ce185799
# Push it to GitHub
git push --set-upstream origin backport-1417-to-develop/v19.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-develop/v19.x

Then, create a pull request where the base branch is develop/v19.x and the compare/head branch is backport-1417-to-develop/v19.x.

paulgessinger added a commit to paulgessinger/acts that referenced this pull request Sep 20, 2022
Previously, the vertex performance writer would assume that there's a 1:1 correspondence between fitted tracks and truth particles. This prevented to run it on track finding output, where we have many more tracks, esp. without ambiguity resolution.

This PR implements hit based truth matching between tracks and particles and uses that to define truth vertices, etc.
I have to change the minimum track fraction to consider a vertex truth matched drastically, which I'm guessing is because of the aforementioned high duplicate count: if you have a ton of tacks, even correctly reconstructed tracks will likely have a bunch of fake tracks attached.

I also switched the physmon particle gun to 201 vertices with beamspot smearing (but the vertexing still runs without beamspot constraint). The IVF gives something like 50 vertices per event, while the AMVF only finds a handful.

Finally, I added a script to make histograms from the performance ntuple and now feed that into the physmon histogram comparison.
paulgessinger added a commit that referenced this pull request Sep 22, 2022
…9.x] (#1536)

Previously, the vertex performance writer would assume that there's a
1:1 correspondence between fitted tracks and truth particles. This
prevented to run it on track finding output, where we have many more
tracks, esp. without ambiguity resolution.

This PR implements hit based truth matching between tracks and particles
and uses that to define truth vertices, etc. I have to change the
minimum track fraction to consider a vertex truth matched drastically,
which I'm guessing is because of the aforementioned high duplicate
count: if you have a ton of tacks, even correctly reconstructed tracks
will likely have a bunch of fake tracks attached.

I also switched the physmon particle gun to 201 vertices with beamspot
smearing (but the vertexing still runs without beamspot constraint). The
IVF gives something like 50 vertices per event, while the AMVF only
finds a handful.

Finally, I added a script to make histograms from the performance ntuple
and now feed that into the physmon histogram comparison.
@paulgessinger paulgessinger modified the milestones: next, v20.2.0 Sep 23, 2022
@AJPfleger AJPfleger linked an issue May 22, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport develop/v19.x Backport this PR to the v19.x series
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add monitoring for vertexing in physmon
3 participants