cmssw icon indicating copy to clipboard operation
cmssw copied to clipboard
verified publisher icon cms-sw

Follow-up tasks for the LST algorithm

Open VourMa opened this issue 1 year ago • 12 comments

This issue is a simplified version of SegmentLinking/cmssw#75, meant to list and keep track of the various upcoming developments for the LST algorithm that were either brought up during the PR #45117 review or planned by the development team. The order of the tasks below roughly corresponds to their priority/timescale, as estimated by the developers.

  • [x] Integrate accumulated algorithm developments held back during integration PR review (SegmentLinking/cmssw#117).

    More details

    • [x] Update LST README, printouts and minor fixes:

      • Covered partially by SegmentLinking/cmssw#55. A follow up to accommodate the developments made in SegmentLinking/cmssw#72 will be needed.
      Links to specific comments
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1681737544
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1681741075
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1833259595
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1833261173
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1844301482
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1844313384

    • [x] Deal with the configurability of the pT threshold:

      • Covered by SegmentLinking/cmssw#39.
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749245650

  • [x] LST in HLT

    More details
    • [x] Rebase and merge SegmentLinking/cmssw#107.

  • [x] Changes in LST conditions data

    More details
    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749249842: I would prefer if the possibility of overriding the conditions data setting LST_BASE or TRACKLOOPERDIR would be disabled when building this code inside CMSSW.
    • [x] Remove lst_INF
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1625114981
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749241840
    • [x] https://github.com/cms-sw/cmssw/pull/45117#issuecomment-2479912047: LSTESData::geometryDataDir needs a check that getenv("CMSSW_SEARCH_PATH") is not null.

  • [ ] LST workflow

    More details
    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1664589679: Respect new relval_Run4.py structure.
    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682593922: Decide if we want different workflows for different backends.
    • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682597612: Implement a "GPU vs CPU" workflow.
    • [x] https://github.com/cms-sw/cmssw/pull/45117#issuecomment-2479912047: Fill seed stop info when LST runs (expected by DQM/validation).

  • [x] Make general-purpose functions widely available in CMSSW

    More details

    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1674782508

    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749143439

    • The plan would be to move all general-purpose functions (work started in SegmentLinking/cmssw#71):

      • Follow up PR with general functions.
      • Follow up PR moving LST to use those and delete custom implementations.

  • [x] Rewrite the loops in the kernels using uniform_elements, independent_groups, independent_group_elements, etc.

    More details

    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749244105: Experiment with make_workdiv.

    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682849644: No need to set the grid size to 1, the alpaka execution will run the blocks one after the other automatically. You can set the grid size to 1 - the difference will be in how the kernel are executed over the elements. Either way, I think whatever approach you choose should be documented.

    • [x] https://github.com/cms-sw/cmssw/pull/45117#pullrequestreview-2286131239

      Links to specific comments
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747147790
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747148816
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747157117
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747159610
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747160439
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747161690
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747492866
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747493175
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747493376
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747497128
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747501157
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747514758
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747514956
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747515175
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747519657
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747520652
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748006364
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748008415
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748008598
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748008657
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748008802
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009215
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009259
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009459
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009869
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748010016
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748010582
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748010837
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748015075
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748015269
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748058899
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749143810
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749144167
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749163618
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749163821
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749164934
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749165613
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749236516
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749236618
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749236663
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749237704
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749237801
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238020
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238089
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238179
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238234
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238443
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238487

  • [x] Rewrite kernels with proper, concrete dimensions instead of templated types

    More details

    • [x] https://github.com/cms-sw/cmssw/pull/45117#pullrequestreview-2286131239:

      Links to specific comments
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747146697
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747161172
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747493770
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747495326
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747501517
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747519297
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747520469
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748005531
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748005961
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748006364
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748008415
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748008961
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009215
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009259
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009395
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009581
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748009869
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748010016
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748010483
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748010582
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748010772
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748013396
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748058899
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749143631
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749143810
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749143984
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749144167
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749163156
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749163618
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749163821
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749164831
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749165439
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749236366
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749237268
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749237704
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749237801
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749237960
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238020
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238089
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238179
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238234
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238303
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238386
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238443
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749238487
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1833303542

  • [ ] Improvements to the data format interface between LST and CMSSW

    More details

    • [x] https://github.com/cms-sw/cmssw/pull/45117#pullrequestreview-2289657004: Define the LST inputs as PortableCollections, and pass them (or their Views) to the LST algorithm.

      Links to specific comments
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682835338: Fill a host SoA directly in LSTPhase2OTHitsInputProducer, copy it to device, and avoid the 6 intermediate copies of the std::vectors.

    • [ ] https://github.com/cms-sw/cmssw/pull/45117#pullrequestreview-2289657004: Define the LST output as PortableCollections, and pass them (or their Views) to the LST algorithm.

      Links to specific comments
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1718975196: Performance-wise best would be to move them in LSTProducer::produce(), that would require the LST::hits() etc to either return a mutable reference, or a value that was moved-from in the hits() method itself.
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1718978133: Eventually (or ideally) we'd want to issue the alpaka::memcpy() calls to copy the data from device to host in acquire(), and have the destination host memory accessed only in produce().

  • [ ] Construct LST condition data from the existing EventSetup info

    More details
    • [ ] https://github.com/cms-sw/cmssw/pull/45117#pullrequestreview-2286131239:
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749373735
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#issuecomment-2480475750

  • [ ] Improvements in storing and using "magic numbers

    More details

    • [ ] https://github.com/cms-sw/cmssw/pull/45117#pullrequestreview-2286131239:

      Links to specific comments
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747431534
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1747438638
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748017178
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748057472
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748057673
      • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1748057925

  • [x] Miscellaneous improvements

    More details
    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749139063: Try std::binary_search(data, data + ndata, search_val) from C++20.
    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749241199: Review uint4 usage.

  • [ ] Improvements to ESProducer

    More details
    • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1699051274: Make it more clear which members are on the device memory and which are on the host memory. Suggest to consider moving all data that are used only in host code into a separate ES data product.

FYI @slava77 @ariostas

VourMa avatar Nov 20 '24 14:11 VourMa

cms-bot internal usage

cmsbuild avatar Nov 20 '24 14:11 cmsbuild

A new Issue was created by @VourMa.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

cmsbuild avatar Nov 20 '24 14:11 cmsbuild

assign reconstruction, heterogeneous, alca, hlt, upgrade

makortel avatar Nov 20 '24 14:11 makortel

@cms-sw/tracking-pog-l2

makortel avatar Nov 20 '24 14:11 makortel

New categories assigned: reconstruction,heterogeneous,alca,hlt,upgrade

@atpathak,@consuegs,@fwyzard,@jfernan2,@makortel,@mandrenguyen,@Martin-Grunewald,@mmusich,@Moanwar,@perrotta,@srimanob,@subirsarkar you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild avatar Nov 20 '24 14:11 cmsbuild

  • item "LST in HLT" has been addressed at https://github.com/cms-sw/cmssw/pull/46828

mmusich avatar Dec 04 '24 10:12 mmusich

The following items have been addressed by https://github.com/cms-sw/cmssw/pull/46857 and have been marked as done in the description:

  • [x] Integrate accumulated algorithm developments held back during integration PR review

    • [x] Update LST README, printouts and minor fixes
    • [x] Deal with the configurability of the pT threshold

  • [ ] Changes in LST conditions data

    • [x] LSTESData::geometryDataDir needs a check that getenv("CMSSW_SEARCH_PATH") is not null.

VourMa avatar Dec 04 '24 11:12 VourMa

The following items have been addressed by https://github.com/cms-sw/cmssw/pull/46967 and have been marked as done in the description:

  • [ ] LST workflow

    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1664589679: Respect new relval_Run4.py structure.
    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682593922: Decide if we want different workflows for different backends.
    • [ ] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682597612: Implement a "GPU vs CPU" workflow.
    • [x] https://github.com/cms-sw/cmssw/pull/45117#issuecomment-2479912047: Fill seed stop info when LST runs (expected by DQM/validation).

  • [ ] Miscellaneous improvements

    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749139063: Try std::binary_search(data, data + ndata, search_val) from C++20.

VourMa avatar Dec 23 '24 12:12 VourMa

With the merging of #47033, the following items have been addressed and marked as done in the description:

  • [x] Make general-purpose functions widely available in CMSSW

VourMa avatar Jan 30 '25 10:01 VourMa

With the merging of #47084, the following items have been addressed and marked as done in the description:

  • [x] Rewrite the loops in the kernels using uniform_elements, independent_groups, independent_group_elements, etc.
  • [x] Rewrite kernels with proper, concrete dimensions instead of templated types

VourMa avatar Jan 30 '25 10:01 VourMa

With the merging of https://github.com/cms-sw/cmssw/pull/47793, the following items have been addressed and marked as done in the description:

  • [ ] Improvements to the data format interface between LST and CMSSW
    • [x] https://github.com/cms-sw/cmssw/pull/45117#pullrequestreview-2289657004: Define the LST inputs as PortableCollections, and pass them (or their Views) to the LST algorithm.
      • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682835338: Fill a host SoA directly in LSTPhase2OTHitsInputProducer, copy it to device, and avoid the 6 intermediate copies of the std::vectors.

The top level task has not been marked as done yet, because we need to also migrate the LST output also to SoA (the separation between input and output has been made more obvious in terms of subtasks). This will be looked at in the next couple of weeks.

VourMa avatar May 11 '25 10:05 VourMa

With the merging of https://github.com/cms-sw/cmssw/pull/48285, the following items have been addressed and marked as done in the description:

  • [x] Miscellaneous improvements
    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1749241199: Review uint4 usage.

VourMa avatar Jun 12 '25 11:06 VourMa

With the merging of #48409, the following items have been addressed. @VourMa could you mark them as done in the description?

  • [x] Improvements to the data format interface between LST and CMSSW More details

    • [x] CMSSW Integration of LST #45117 (comment): Fill a host SoA directly in LSTPhase2OTHitsInputProducer, copy it to device, and avoid the 6 intermediate copies of the std::vectors.

    • [x] CMSSW Integration of LST #45117 (review): Define the LST output as PortableCollections, and pass them (or their Views) to the LST algorithm. Links to specific comments

    • [x] CMSSW Integration of LST #45117 (comment): Performance-wise best would be to move them in LSTProducer::produce(), that would require the LST::hits() etc to either return a mutable reference, or a value that was moved-from in the hits() method itself.

    • [x] CMSSW Integration of LST #45117 (comment): Eventually (or ideally) we'd want to issue the alpaka::memcpy() calls to copy the data from device to host in acquire(), and have the destination host memory accessed only in produce().

ariostas avatar Aug 05 '25 13:08 ariostas

With the merging of #48508, the following items have been addressed and marked as done in the description:

  • [x] LST workflow

    • [x] https://github.com/cms-sw/cmssw/pull/45117#discussion_r1682597612: Implement a "GPU vs CPU" workflow.

VourMa avatar Aug 13 '25 06:08 VourMa