Skip to content

[15_0_X] Optimizations for the MkFit converters #47927

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

Open
wants to merge 2 commits into
base: CMSSW_15_0_X
Choose a base branch
from

Conversation

elusian
Copy link

@elusian elusian commented Apr 22, 2025

PR description:

Tracking with mkFit depends on a series of converters that translate the standard CMSSW objects (e.g. tracking rechits) to the mkFit specific formats.
Given the introduction of mkFit for the tracking iteration at HLT in 2025, it's important that all these converters are as fast as possible.

This PR includes two commits:

  • the first adds an output to the hit converters that details the number of hits for each layer, which can be used as an hint for allocations in further steps
  • the second adds a parameter in the standard SiStripRecHitConverterAlgorithm that allows filling further collections and a new plugin (MkFitSiStripHitConverterFromClusters), which acts as drop-in replacement for both SiStripRecHitConverter and MkFitSiStripHitConverter while being faster than the sum of the two.

Only the first commit touches code that is currently being executed in release (but it amounts to a call to reserve on some vectors), while the second is fully opt in and needs to be added to sequences to be active.
To avoid touching existing code (for a faster review) there is a small amount of code repetition in convertHits.h that can be corrected with a future PR.

PR validation:

This change was validated on 731 TTbar MC events from the Winter25 campaign.
The results can be found here.
No differences were observed in either of the commits.
No other code physics object depend on the modules that are being modified.

The timing improvement was measured in the HLT timing server:

  • Baseline: 477.8 ms, 540.3 +/- 3 ev/s
  • This PR: 467.3 ms, 552.9 +/- 1.7 ev/s (+2.3 +/- 0.6% throughput)

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Backport of #47926 for 15_0_X

cc @mmasciov @kskovpen

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 22, 2025

A new Pull Request was created by @elusian for CMSSW_15_0_X.

It involves the following packages:

  • RecoLocalTracker/SiStripRecHitConverter (reconstruction)
  • RecoTracker/MkFit (reconstruction)
  • RecoTracker/MkFitCore (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @VinInn, @VourMa, @alesaggio, @dgulhan, @echabert, @felicepantaleo, @gbenelli, @gpetruc, @jlidrych, @makortel, @missirol, @mmusich, @mtosi, @robervalwalsh, @rovere, @threus, @yduhm this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 22, 2025

cms-bot internal usage

@mmusich
Copy link
Contributor

mmusich commented Apr 22, 2025

backport of #47926

@mmusich
Copy link
Contributor

mmusich commented Apr 22, 2025

allow @elusian test rights

@mmusich
Copy link
Contributor

mmusich commented Apr 24, 2025

@elusian #47929 was just merged, please do the same here as suggested in the master PR ( #47926 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants