Move LSTGeometry repo into ESProducer by ariostas · Pull Request #203 · SegmentLinking/cmssw

ariostas · 2025-09-26T15:32:52Z

This is still a work in progress, but I'm opening a draft PR so you can track the progress.

This is relatively close to working for standalone, but I still have to figure out the ESProducer part.

slava77 · 2025-09-26T22:41:10Z

I expect that the final code will not use a csv file to generate the geometry bin/txt file.
RecoTracker/MkFit/plugins/MkFitGeometryESProducer.cc and the standalone representation will be derived similar to RecoTracker/MkFit/test/DumpMkFitGeometry.cc
... at least as an inspiration

ariostas · 2025-11-03T20:26:58Z

After binning, it takes about 15 seconds, which is starting to be reasonable. There are more improvements that can be made to make it faster, but for now I'll switch gears to get the CMSSW part in place.

ariostas · 2025-12-03T17:26:59Z

Not sure if it will work, but let's see what happens.

/run cmssw

github-actions · 2025-12-03T18:24:54Z

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

github-actions · 2025-12-03T19:13:33Z

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

github-actions · 2025-12-03T20:21:30Z

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

github-actions · 2025-12-04T17:42:11Z

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

OOTB All Tracks

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

ariostas · 2025-12-04T17:52:49Z

The plots actually look better than I expected. I'll take another look at the standalone comparison and post it here. I'll also do some cleanup and look into what could be the source of the differences in the |eta|~1-2 range.

ariostas · 2025-12-05T22:06:28Z

/run cmssw

Copilot

Pull request overview

This PR refactors the LST (Line Segment Tracking) geometry handling by moving geometry computation logic into a dedicated structure and making it available through both an ESProducer (for CMSSW integration) and a standalone binary. The key changes include:

Introduction of a new LSTGeometry data structure containing centroids, slopes, pixel maps, and module connections
New geometry computation methods using Eigen for matrix operations
An ESProducer (LSTGeometryESProducer) that generates geometry from TrackerGeometry
A standalone binary (lst_make_geometry) for offline geometry generation
Integration with existing LST code through new overloaded methods

Key Changes

New geometry computation framework with multiple helper classes (Module, DetectorGeometry, etc.)
Refactored LSTESData to accept both file-based and LSTGeometry-based initialization
Updated parameter handling to use string-based ptCut labels for ES record lookups

Reviewed changes

Copilot reviewed 41 out of 42 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`RecoTracker/LSTCore/plugins/LSTGeometryESProducer.cc`	New ESProducer for generating LSTGeometry from TrackerGeometry
`RecoTracker/LSTCore/standalone/bin/lst_make_geometry.cc`	New standalone binary for geometry generation
`RecoTracker/LSTCore/interface/LSTGeometry/*.h`	New geometry data structures and computation methods
`RecoTracker/LSTCore/src/LSTESData.cc`	Added overload for LSTGeometry-based initialization
`RecoTracker/LSTCore/src/TiltedGeometry.cc`	Added overload to load from LSTGeometry slopes
`RecoTracker/LSTCore/src/EndcapGeometry.cc`	Added overload to load from LSTGeometry slopes
`RecoTracker/LSTCore/test/DumpLSTGeometry.cc`	New test analyzer for dumping geometry
`RecoTracker/LST/plugins/alpaka/LSTProducer.cc`	Updated ptCut parameter handling
`RecoTracker/LSTCore/standalone/Makefile`	Added build rules for geometry binary

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2025-12-05T23:04:51Z

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

slava77 · 2025-12-05T23:43:03Z

is the copilot reviewer feature something to be manually added? I don't see this option in #212

ariostas · 2025-12-06T00:42:31Z

@slava77 I'm not sure if you need pro or if it's free for everyone (although you can get pro for free by being in education). I'll trigger it for #212

github-actions · 2025-12-06T01:27:33Z

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

OOTB All Tracks

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

ariostas · 2025-12-18T16:20:24Z

/run cmssw

github-actions · 2025-12-18T18:07:06Z

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

OOTB All Tracks

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

ariostas · 2026-01-06T19:10:22Z

/run cmssw lowpt

ariostas · 2026-01-06T19:10:37Z

/run cmssw

github-actions · 2026-01-06T19:53:08Z

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

github-actions · 2026-01-06T21:45:05Z

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

OOTB All Tracks

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

github-actions · 2026-04-06T16:59:06Z

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.7    120.6    126.3     45.0    706.6      9.8     39.4     68.2    207.2      0.2    1444.6     707.3+/- 178.6     470.4   explicit[s=4] (target branch)
   avg     28.1     88.0    122.7    126.2     49.3    706.1     11.2     39.6     67.6    206.2      0.2    1445.2     711.0+/- 183.9     476.8   explicit[s=4] (this PR)

Copilot

Pull request overview

Copilot reviewed 48 out of 49 changed files in this pull request and generated 11 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ariostas · 2026-04-06T21:10:26Z

I applied some of the copilot suggestions. I didn't touch the phi computation because it is set up in a weird way and it doesn't work if you simplify it to a more common expression. I'll run the CI one last time before squashing and opening a PR upstream.

ariostas · 2026-04-06T21:10:49Z

run-ci: [cmssw, checks]

github-actions · 2026-04-06T22:50:18Z

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.

slava77

a few minor edits to consider

ariostas · 2026-04-08T18:30:16Z

run-ci: all

github-actions · 2026-04-08T18:54:01Z

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     31.2     91.7    122.5    128.6     46.2    705.2      9.8     40.1     70.3    207.1      0.1    1452.8     716.4+/- 181.9     475.3   explicit[s=4] (target branch)
   avg     28.3     87.7    122.5    129.2     50.6    702.0     11.1     40.6     70.2    205.7      0.2    1448.1     717.8+/- 186.0     481.9   explicit[s=4] (this PR)

github-actions · 2026-04-08T20:05:49Z

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.

ariostas · 2026-04-16T18:54:01Z

run-ci: [all, hlt]

github-actions · 2026-04-16T19:13:46Z

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     31.0     91.2    121.2    125.1     44.3    703.5      9.8     39.4     67.7    206.9      0.2    1440.2     705.7+/- 177.8     471.1   explicit[s=4] (target branch)
   avg     28.5     87.7    119.9    125.4     49.1    709.7     11.1     39.6     68.0    207.5      0.1    1446.6     708.4+/- 181.3     474.8   explicit[s=4] (this PR)

github-actions · 2026-04-16T20:35:32Z

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-16T20:58:54Z

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.

ariostas · 2026-04-20T19:19:29Z

run-ci: [cmssw, hlt]

github-actions · 2026-04-20T20:38:46Z

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-20T20:53:00Z

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.

slava77 · 2026-04-23T18:49:54Z

@ariostas
I have a hypothesis to test: use nsys profile on a job with 16 threads (as in the timing tests; or less if the profile timeline is not manageable with 16); run it with the old and new setup.

IIUC, the new LSTGeometryESProducer is much slower than the older version from files. If that's the case, more GPU modules unrelated to LST/tracking might contribute device data products or be already running on the GPU compared to the older setup. Also it's more likely that all 16 LST modules across different streams will start running simultaneously (in the old setup chances are that the earlier patatrack tracking modules with some variance in time to completion spread out the start time of the LST modules).

The profiler timeline should clarify if we suffer from accumulation of some other work or if it's mostly from multiple LST starting at the same time.

ariostas · 2026-04-23T19:42:54Z

@slava77 yeah, from looking at nvidia-smi I suspect something like that is happening. With the old setup things there's more variance in the streams, so some of them end up finishing before others reach peak vram usage, whereas with the new setup all streams finish more or less at the same time, so peak vram usage is higher.

One thing I've been trying to figure out is why with multiple streams the vram usage is so much higher. I expected that each stream should use about the same vram as if you ran everything in one stream. However, they end up using more than twice of the single stream peak. I was thinking that the caching allocator was causing some memory fragmentation, so I was experimenting with allocating memory in fixed block sizes, but it didn't really make a difference.

I'll run the profiler for the new and old setups, using 1 and 16 streams, and let you know what I find. Thanks!

slava77 · 2026-04-23T22:04:22Z

from looking at nvidia-smi I suspect something like that is happening. With the old setup things there's more variance in the streams, so some of them end up finishing before others reach peak vram usage, whereas with the new setup all streams finish more or less at the same time, so peak vram usage is higher.

if there's some LST kernel herding/crowding, perhaps a solution is to write some dummy CPU module that just checks something trivial from the LSTGeometry, and schedule it early in the tracking sequence. It's probably more economical to put it in a beginRun (but then I recall there are some subtleties about declaring ES availability at beginRun vs event)

ariostas marked this pull request as draft September 26, 2025 15:33

ariostas commented Oct 24, 2025

View reviewed changes

Comment thread RecoTracker/LSTCore/interface/LSTGeometry/ModuleMapMethods.h Outdated

slava77 reviewed Oct 24, 2025

View reviewed changes

Comment thread RecoTracker/LSTCore/interface/LSTGeometry/LSTMath.h Outdated

slava77 reviewed Oct 24, 2025

View reviewed changes

Comment thread RecoTracker/LSTGeometry/src/ModuleMap.cc

slava77 reviewed Oct 25, 2025

View reviewed changes

Comment thread RecoTracker/LSTGeometry/src/ModuleMap.cc Outdated

ariostas force-pushed the ariostas/lst_geometry branch from c2d0f53 to a5accdb Compare November 4, 2025 19:07

ariostas force-pushed the ariostas/lst_geometry branch from a91bad2 to 14afa6a Compare December 3, 2025 19:50

ariostas requested a review from Copilot December 5, 2025 22:06

Copilot started reviewing on behalf of ariostas December 5, 2025 22:06 View session

Copilot AI reviewed Dec 5, 2025

View reviewed changes

ariostas mentioned this pull request Dec 6, 2025

LST T5-T5 Duplicate Merging #212

Merged

ariostas requested a review from Copilot April 6, 2026 18:23

Copilot started reviewing on behalf of ariostas April 6, 2026 18:24 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

slava77 reviewed Apr 7, 2026

View reviewed changes

Comment thread RecoTracker/LST/plugins/alpaka/LSTModulesDevESProducer.cc Outdated

Comment thread RecoTracker/LSTGeometry/src/Sensor.cc Outdated

ariostas force-pushed the ariostas/lst_geometry branch from d7a0ed3 to e612f24 Compare April 7, 2026 15:48

slava77 mentioned this pull request Apr 14, 2026

updates to handle pLS with OT hits #253

Open

ariostas force-pushed the ariostas/lst_geometry branch from 59f2ef5 to 4e3de58 Compare April 20, 2026 19:11

ariostas changed the title ~~Move LSTGeometry repo into ESProducer and standalone binary~~ Move LSTGeometry repo into ESProducer Apr 20, 2026

ariostas added 4 commits May 22, 2026 05:39

Added LSTGeometry package and associated ESProducer

f954f8e

Reduce memory usage

8b385b9

Switched to argparse and other minor tweaks

c399d2f

Tighten module maps

46de12c

ariostas force-pushed the ariostas/lst_geometry branch from 4e3de58 to 46de12c Compare May 22, 2026 12:52

Conversation

ariostas commented Sep 26, 2025

Uh oh!

slava77 commented Sep 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ariostas commented Nov 3, 2025

Uh oh!

ariostas commented Dec 3, 2025

Uh oh!

github-actions Bot commented Dec 3, 2025

Uh oh!

github-actions Bot commented Dec 3, 2025

Uh oh!

github-actions Bot commented Dec 3, 2025

Uh oh!

github-actions Bot commented Dec 4, 2025

Uh oh!

ariostas commented Dec 4, 2025

Uh oh!

ariostas commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Dec 5, 2025

Uh oh!

slava77 commented Dec 5, 2025

Uh oh!

ariostas commented Dec 6, 2025

Uh oh!

github-actions Bot commented Dec 6, 2025

Uh oh!

ariostas commented Dec 18, 2025

Uh oh!

github-actions Bot commented Dec 18, 2025

Uh oh!

ariostas commented Jan 6, 2026

Uh oh!

ariostas commented Jan 6, 2026

Uh oh!

github-actions Bot commented Jan 6, 2026

Uh oh!

github-actions Bot commented Jan 6, 2026

Uh oh!

github-actions Bot commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ariostas commented Apr 6, 2026

Uh oh!

ariostas commented Apr 6, 2026

Uh oh!

github-actions Bot commented Apr 6, 2026

Uh oh!

slava77 left a comment