Simplify LST input & output by VourMa · Pull Request #252 · SegmentLinking/cmssw

VourMa · 2026-04-06T18:22:57Z

This PR is fully technical, resulting in the same physics performance (validation plots here) and the following timing reduction (baseline timing vs. PR timing):

hltInitialStepSeedTracksLST: 4.6 → 0 ms (-100%);
hltInputLST: 18.5 → 16.2 ms (-12%);
LSTOutputConverter: 67.9 → 55.4 ms (-18%);
Total "Tracking" time reduction (GPU): 717.0 → 673.9 ms (-6%)

slava77 · 2026-04-17T07:55:25Z

+        std::vector<Hit> hits;
+        for (unsigned int iHit = 0, nHits = proto.recHitsSize(); iHit < nHits; ++iHit) {
+          TrackingRecHitRef refHit = proto.recHit(iHit);
+          if (refHit->isValid())
+            hits.push_back((Hit)&(*refHit));
+        }
+        sort(hits.begin(), hits.end(), HitLessByRadius());


sorting and invalid hit filtering should probably be configurable and done regardless of removeOTRechits_

These lines are a direct copy from:

cmssw/RecoTracker/TkSeedGenerator/plugins/SeedGeneratorFromProtoTracksEDProducer.cc

Lines 161 to 167 in 8f4f2fa

std::vector<Hit> hits;

for (unsigned int iHit = 0, nHits = proto.recHitsSize(); iHit < nHits; ++iHit) {

TrackingRecHitRef refHit = proto.recHit(iHit);

if (refHit->isValid())

hits.push_back((Hit) & (*refHit));

}

sort(hits.begin(), hits.end(), HitLessByRadius());

so this part just replicates the behavior from the code below.

Would you like me to test what happens if I don't apply this sorting+filtering?

The only case that this sorting+filtering is not applied is when useProtoTrackKinematics and not removeOTRechits_. But I wouldn't like to change this general behavior, as it will have effects to the Run 3 reconstruction potentially.

I implied that after my updates removeOTRechits_ can be set to false, then we'd likely want to have the same filtering and sorting still present here

This is now implemented in 98bb453. Could you take a look and confirm this is how you wanted it?

slava77 · 2026-04-17T07:59:22Z

+        if (hits.size() > 2) {
+          SeedFromProtoTrack seedFromProtoTrack(config_, proto, SeedingHitSet(hits), es);
+          if (seedFromProtoTrack.isValid())
+            (*result).push_back(seedFromProtoTrack.trajectorySeed());
+        }


as a part of debugging the efficiency loss: please check if this part ever fails to produce seeds (IIUC the number of pixel hits is still expected to be at least 3, the question is if the SeedFromProtoTrack construction sometimes makes invalid seeds

I can check this. However, since the part when we switch to this reconstruction is not enabled in this PR now, is it OK if we go ahead we made the PR to cms-sw first and check this in parallel?

VourMa · 2026-04-24T12:10:06Z

run-ci: [all, hlt]

github-actions · 2026-04-24T12:31:00Z

The PR was built and ran successfully in standalone mode running on CPU. Here are some of the comparison plots.

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     30.7     90.3    119.2    125.0     44.3    705.0      9.8     39.5     67.6    206.6      0.2    1438.1     702.5+/- 181.5     477.2   explicit[s=4] (target branch)
   avg     31.0     91.4    122.5    125.5     44.7    709.8      9.8     39.3     68.0    206.2      0.1    1448.4     707.5+/- 180.2     478.3   explicit[s=4] (this PR)

github-actions · 2026-04-24T13:07:44Z

There was a problem while building and running with CMSSW. The logs can be found here.

github-actions · 2026-04-24T14:00:25Z

The PR was built and ran successfully with HLT setup running on CPU (procModifiers = ). Here are some plots.

HLT General Plots

The full set of validation and comparison plots can be found here.

VourMa · 2026-04-24T17:27:05Z

run-ci: cmssw

github-actions · 2026-04-24T17:49:15Z

There was a problem while building and running with CMSSW. The logs can be found here.

VourMa · 2026-04-24T17:53:12Z

run-ci: cmssw

github-actions · 2026-04-24T19:41:05Z

The PR was built and ran successfully with CMSSW running on CPU. Here are some plots.

OOTB All Tracks

The full set of validation and comparison plots can be found here.

…TrackKinematics

VourMa · 2026-04-24T21:38:09Z

@slava77 Let me know whether I can push this to CMSSW.

slava77 · 2026-04-24T22:01:58Z

(not for this PR, but something to consider for a cleanup of the configs spaghetti , or is it copy-pasta)
I'd import from a reference definition HLTrigger/Configuration/python/HLT_75e33/modules/hltInitialStepTrajectorySeedsLST_cfi.py and clone with just a few changes

I think this is "against the HLT config conventions", from the point of view that modules need to be explicitly defined or be modified clones of other modules in the same file. I can ask Marco M. at some point whether it would be acceptable though.

slava77 · 2026-04-24T22:27:01Z


-    TrajectorySeedCollection seeds;
+    seeds.clear();


did this give some speedup?
The seeds are needed only between L249 and L262
Same for the hitsForSeed.

It's a bit harder to read/follow than if it's just in the scope where it's used.

it may be more clear if seeds.clear(); is called just before it's used/populated

The speed up from this was negligible. The current implementation changed the lines as close as possible to the previous lines but I can move this line close to 249 to make the logic clearer.

- Add flags for tracking/seeding production - Conditional creation of products - Per-iteration heap allocations moved out of the loop - Remove unnecessary OT hit loop + sort for pLS candidates

VourMa · 2026-04-25T12:55:26Z

run-ci: [all, hlt]

github-actions · 2026-04-25T13:15:10Z

The PR was built and ran successfully in standalone mode running on CPU. Here are some of the comparison plots.

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     30.7     90.5    120.7    126.5     45.2    705.5      9.9     39.7     69.3    207.1      0.1    1445.2     709.0+/- 177.1     473.7   explicit[s=4] (target branch)
   avg     30.9     91.4    121.0    126.9     45.4    707.4      9.9     39.8     70.0    206.7      0.7    1450.2     711.8+/- 184.1     477.9   explicit[s=4] (this PR)

github-actions · 2026-04-25T14:17:41Z

The PR was built and ran successfully with CMSSW running on CPU. Here are some plots.

OOTB All Tracks

The full set of validation and comparison plots can be found here.

github-actions · 2026-04-25T14:26:22Z

The PR was built and ran successfully with HLT setup running on CPU (procModifiers = ). Here are some plots.

HLT General Plots

The full set of validation and comparison plots can be found here.

VourMa · 2026-04-27T17:17:49Z

@slava77 Is it OK to submit this to CMSSW?

slava77

Thanks for the update.
It looks to me ready to go upstream/cms-sw

Remove redundant tracking modules

d35a0eb

VourMa force-pushed the simplifyLSTInputOutput branch from 72ae121 to 8f4f2fa Compare April 6, 2026 21:35

VourMa changed the title ~~Simplify lst input output~~ Simplify LST input & output Apr 7, 2026

slava77 reviewed Apr 17, 2026

View reviewed changes

Comment thread RecoTracker/LST/plugins/LSTOutputConverter.cc Outdated

slava77 reviewed Apr 17, 2026

View reviewed changes

Comment thread RecoTracker/LST/plugins/LSTOutputConverter.cc Outdated

slava77 reviewed Apr 17, 2026

View reviewed changes

VourMa force-pushed the simplifyLSTInputOutput branch from 8f4f2fa to 73365bd Compare April 24, 2026 11:50

VourMa added 2 commits April 24, 2026 14:31

Remove hltInitialStepSeedTracksLST

baaa7c2

Implement removeOTRechits and sorting+filtering of hits when useProto…

8ba56c9

…TrackKinematics

VourMa force-pushed the simplifyLSTInputOutput branch from 9b8e9aa to 9b32edd Compare April 24, 2026 21:37

slava77 reviewed Apr 24, 2026

View reviewed changes

Improvements for the LST output

0c81f3f

- Add flags for tracking/seeding production - Conditional creation of products - Per-iteration heap allocations moved out of the loop - Remove unnecessary OT hit loop + sort for pLS candidates

VourMa force-pushed the simplifyLSTInputOutput branch from 9b32edd to 0c81f3f Compare April 25, 2026 12:55

slava77 approved these changes Apr 27, 2026

View reviewed changes

	std::vector<Hit> hits;
	for (unsigned int iHit = 0, nHits = proto.recHitsSize(); iHit < nHits; ++iHit) {
	TrackingRecHitRef refHit = proto.recHit(iHit);
	if (refHit->isValid())
	hits.push_back((Hit) & (*refHit));
	}
	sort(hits.begin(), hits.end(), HitLessByRadius());

Conversation

VourMa commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VourMa commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

VourMa commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

VourMa commented Apr 24, 2026

Uh oh!

github-actions Bot commented Apr 24, 2026

Uh oh!

VourMa commented Apr 24, 2026

Uh oh!

slava77 Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VourMa commented Apr 25, 2026

Uh oh!

github-actions Bot commented Apr 25, 2026

Uh oh!

github-actions Bot commented Apr 25, 2026

Uh oh!

github-actions Bot commented Apr 25, 2026

Uh oh!

VourMa commented Apr 27, 2026

Uh oh!

slava77 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

VourMa commented Apr 6, 2026 •

edited

Loading

slava77 Apr 24, 2026 •

edited

Loading