Add LST and LSTCore #30

ariostas · 2024-05-29T16:21:44Z

Here's a draft of the LSTCore package, but now using real files. Here are some notes about it.

The data directory is temporary and should be removed before we submit the PR to the main CMSSW repo. We should get started populating RecoTracker-LSTCore.
I added a standalone directory, following Slava's suggestion. I adapted the scripts so that you can compile and run as before. One important thing to note is that you need to run setup.sh right before compiling/running (we can make it more seamless later).
I made the LSTCore commit be co-authored by all contributors to TrackLooper so that we all can get some credit once it goes into CMSSW.

I tested running both standalone and CMSSW, and everything seems fine.

ariostas · 2024-05-29T16:26:27Z

This is a replacement of #29. This is still a draft since it includes all the data files, which should be removed. There's also many files that were added just for the standalone part, and I'm not sure if that's fine.

VourMa · 2024-05-29T16:38:27Z

Will check it out tonight and maybe test it and report back. Thanks!

slava77 · 2024-05-29T17:02:32Z

The data directory is temporary and should be removed before we submit the PR to the main CMSSW repo. We should get started populating RecoTracker-LSTCore.

these will likely trigger an issue of increasing the git repo by too much and will need to be removed from the git history.

Is it a matter of getting CI to know where to take the data files?
We could already populate the master in a fork of cms-data in https://github.com/SegmentLinking/RecoTracker-LSTCore and just have the CI do git clone https://github.com/SegmentLinking/RecoTracker-LSTCore.git $CMSSW_BASE/src/RecoTracker/LSTCore/data

ariostas · 2024-05-29T18:43:25Z

these will likely trigger an issue of increasing the git repo by too much and will need to be removed from the git history.

Yeah, I removed them from the git history

Is it a matter of getting CI to know where to take the data files?

Yeah, I'll do that. Thanks!

VourMa

I foresee a comment at the review about the many commits with "invalid states", e.g. when the LST was included as an external package, something that was/will never be useful for CMSSW. Based on this, shall we squash the commits? Maybe two commits would make sense, the one that Added LSTCore and one with the rest of the commits in the cmssw repo.

RecoTracker/FinalTrackSelectors/plugins/TrackListMerger.cc

RecoTracker/LST/src/alpaka/ES_ModulesDev.cc

RecoTracker/LST/test/LSTAlpakaTester.py

RecoTracker/LST/test/LSTInputTester.py

RecoTracker/LSTCore/README.md

RecoTracker/LSTCore/interface/alpaka/Constants.h

RecoTracker/LSTCore/src/alpaka/Triplet.h

VourMa · 2024-05-30T16:33:52Z

RecoTracker/LSTCore/standalone/setup.sh

+else
+  export SCRAM_ARCH=el8_amd64_gcc12
+fi
+export CMSSW_VERSION=CMSSW_14_1_0_pre3


Is this ok if we leave it like that? Maybe we should just grab the version of CMSSW we have set up?

it's a standalone setup, somewhat tuned to available input file formats and other dependencies (like a test file format).

Also, aren't the following lines (from cmsrel) implying that the version is not known otherwise?

We should have a better setup script. I only made the minimal modifications to get it to work.

VourMa · 2024-05-30T16:44:20Z

I also tried the standalone version within CMSSW. It compiles fine but it doesn't run because:

ERROR: Could not find the file = "/home/users/evourlio/LSTinCMSSW/movingFullyToCMSSW/CMSSW_14_1_0_pre3/src/RecoTracker/LSTCore/standalone/..//data/OT800_IT615_pt0.8/endcap_orientation.bin"

I guess this is because data files were removed from the history. Are there changes pending for things to work? Or is there some manual intervention required, like I need to clone the data files with an extra command? ~~I see that this is still empty, so I can just push the data folder structure there?~~ I noticed that Andres has already created an initial branch there, that is filled with data.

slava77 · 2024-05-30T17:15:37Z

I also tried the standalone version within CMSSW. ...

we should probably decide what "standalone" would actually mean:

a setup that can compile and run sdl_cuda and such while also being able to run a cmsRun
or something more "restrictive" (with lower dependency stack) using a sparse checkout where only the libsdl_* and sdl_* bins are compiled and can run.

I guess this is because data files were removed from the history. Are there changes pending for things to work?

That's a bit related to the above:

if the standalone will operate inside CMSSW, we can default to what will be visible in $CMSSW_RELEASE_BASE/external/el8_amd64_gcc12/data/RecoTracker/LSTCore/data
or assume some local checkout path; in this case the LST_BASE environment will need to be set

slava77 · 2024-05-30T17:17:15Z

we can default to what will be visible in $CMSSW_RELEASE_BASE/external/el8_amd64_gcc12/data/RecoTracker/LSTCore/data

and if that path is not available, to add a git clone/checkout from https://github.com/SegmentLinking/RecoTracker-LSTCore

RecoTracker/LSTCore/standalone/setup.sh

VourMa · 2024-05-31T13:40:37Z

we should probably decide what "standalone" would actually mean:

a setup that can compile and run sdl_cuda and such while also being able to run a cmsRun

or something more "restrictive" (with lower dependency stack) using a sparse checkout where only the libsdl_* and sdl_* bins are compiled and can run.

In principle both can work with small changes to the current state of the code, I believe.

For the first case, all we need is to modify:

cmssw/RecoTracker/LSTCore/src/alpaka/LSTESData.dev.cc

Lines 14 to 30 in 8ced0f1

    
           std::string trackLooperDir() { 
        
             const char* path_lst_base = std::getenv("LST_BASE"); 
        
             const char* path_tracklooperdir = std::getenv("TRACKLOOPERDIR"); 
        
             std::string path_str; 
        
             if (path_lst_base != nullptr) { 
        
               path_str = path_lst_base; 
        
             } else if (path_tracklooperdir != nullptr) { 
        
               path_str = path_tracklooperdir; 
        
               path_str += "/../"; 
        
             } else { 
        
               // FIXME: temporary solution, will need to pass a value from FileInPath or CMSSW search path 
        
               // in the `LSTProducer` or a related ES producer 
        
               path_str = std::getenv("CMSSW_BASE"); 
        
               path_str += "/src/RecoTracker/LSTCore"; 
        
             } 
        
             return path_str; 
        
           }

to point to a different directory according to your comment:

and if that path is not available, to add a git clone/checkout from https://github.com/SegmentLinking/RecoTracker-LSTCore

For the second case, I did:

git clone --filter=blob:none --no-checkout --depth 1 --sparse --branch CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles https://github.com/SegmentLinking/cmssw.git
cd cmssw/
git sparse-checkout add HeterogeneousCore/AlpakaInterface RecoTracker/LSTCore
git checkout

Given the absence of the $CMSSW_RELEASE_BASE/external/el8_amd64_gcc12/data/RecoTracker/LSTCore/data directory, I did:

cd RecoTracker/LSTCore
git clone git@github.com:SegmentLinking/RecoTracker-LSTCore.git data
cd data
git fetch origin initial
git checkout initial
cd ../../..

Then, the include directories are a bit off, because there is no proper $CMSSW_BASE. I fixed this by including -I../../.. or -I../../../.. in the Makefile and SDL/Makefile respectively. Doing so, I managed to run everything.
Of course, this was a "hack" at this stage. The proper solution to me would be to add a compilation flag to disable the LST_IS_CMSSW_PACKAGE flag, and change lines like:

cmssw/RecoTracker/LSTCore/src/alpaka/Kernels.h

Lines 7 to 10 in 8ced0f1

    
           #else 
        
           #include "Constants.h" 
        
           #include "Module.h" 
        
           #endif

to point to the appropriate directories, as these lines are useless right now, if also the standalone is in CMSSW.

UPDATE:

As of 6025567, the fix of the Makefiles has been applied, so the standalone package can be used without manual interventions.

slava77 · 2024-05-29T17:06:22Z

RecoTracker/LSTCore/interface/alpaka/Constants.h

@@ -0,0 +1,157 @@
+#ifndef Constants_cuh
+#define Constants_cuh


will need compliant header guards; not sure it's an ovekill to do them with #ifdef LST_IS_CMSSW_PACKAGE;
otherwise

Suggested change

#define Constants_cuh

#define RecoTracker_LSTCore_alpaka_Constants_cuh

slava77 · 2024-05-29T17:12:41Z

RecoTracker/LSTCore/standalone/code/core/trktree.cc

@@ -0,0 +1,7007 @@
+#include "trktree.h"
+trktree trk;


is this auto-generated?

@sgnoohc Perhaps you know/remember?

RecoTracker/LST/test/LSTAlpakaTester.py

RecoTracker/LSTCore/interface/alpaka/Constants.h

slava77 · 2024-05-30T16:57:28Z

RecoTracker/LSTCore/standalone/setup.sh

+else
+  export SCRAM_ARCH=el8_amd64_gcc12
+fi
+export CMSSW_VERSION=CMSSW_14_1_0_pre3


it's a standalone setup, somewhat tuned to available input file formats and other dependencies (like a test file format).

Also, aren't the following lines (from cmsrel) implying that the version is not known otherwise?

slava77 · 2024-05-30T22:35:05Z

RecoTracker/LSTCore/src/alpaka/Triplet.h

+    float drt_InSeg = rtMid - rtIn;
+    float dz_InSeg = zMid - zIn;
+    float dr3_InSeg =
+        alpaka::math::sqrt(acc, rtMid * rtMid + zMid * zMid) - alpaka::math::sqrt(acc, rtIn * rtIn + zIn + zIn);


todo: pick up the update from SegmentLinking/TrackLooper#407

Fixed in #44.

ariostas · 2024-05-31T15:35:07Z

I foresee a comment at the review about the many commits with "invalid states"

Yeah, I think it makes sense to squash commits further. Let's also resolve all the discussions here by adding more commits to this branch and once it's in good shape we'll squash everything into 1-2 commits.

ariostas · 2024-05-31T18:25:11Z

Here's a new branch with squashed commits. I can change the branch name and/or commit messages if you prefer.

https://github.com/SegmentLinking/cmssw/tree/LST_and_LSTCore

VourMa · 2024-05-31T18:30:30Z

Here's a new branch with squashed commits. I can change the branch name and/or commit messages if you prefer.

https://github.com/SegmentLinking/cmssw/tree/LST_and_LSTCore

Seems good to me. Should we force-push those commits in this branch, so that we can keep track of the discussions that we have already started here?

ariostas · 2024-05-31T18:40:50Z

Should we force-push those commits in this branch, so that we can keep track of the discussions that we have already started here?

Yeah, sounds good. I wasn't sure about that so I made a separate one to be safe, but let's just stick with this branch for discussions.

ariostas · 2024-05-31T18:43:50Z

There are lots of changes after the force-push, but that's just because I re-synced with upstream master.

slava77 · 2024-05-31T20:56:24Z

There are lots of changes after the force-push, but that's just because I re-synced with upstream master.

the actual diffs in the https://github.com/SegmentLinking/cmssw/pull/30/files look OK.

ariostas · 2024-06-13T04:27:22Z

Should we close this PR now? It seems like it's no longer needed for discussions.

VourMa · 2024-06-13T09:47:35Z

Should we close this PR now? It seems like it's no longer needed for discussions.

I think that the point of this PR is to mirror the ones in CMSSW master, so that when the central one is merged, this is also merged automatically. At least that's what I had understood based on the mkFit model of development.

slava77 · 2024-06-13T12:22:25Z

I think that the point of this PR is to mirror the ones in CMSSW master, so that when the central one is merged, this is also merged automatically. At least that's what I had understood based on the mkFit model of development.

yes, that's the idea.
In addition, it's a convenient place to have aside discussion about changes there.

Also, being able to run our CI here would also be helpful; although the comparisons would have to be with an earlier version of itself; so, perhaps more complicated. Would it be possible to at least run the linter part?

VourMa · 2024-06-28T21:06:47Z

Should I sync the SegmentLinking/cmssw master to the cms-sw/cmssw master? I guess that would allow us to see the conflicts also here?

RecoTracker/LSTCore/interface/alpaka/Constants.h

RecoTracker/LSTCore/src/alpaka/Quintuplet.h

fix `SiStripBadStrip_PayloadInspector` after merging of cms-sw#45795

Updated cms::Exception using modern C++

[DQM] Changes suggested by new llvm18 clang-format

…ang-format [ALCA-UPGRADE] Changes suggested by new llvm18 clang-format

[Phase 2 L1T] fixing ASAN errors in Phase2L1CaloJetEmulator

Phase2-hgx360A Modify the V19 longitudinal structure of HGCal according to Meeting specification of 06/09/2024

rename modifier `run3_2024_L1T` to `stage2L1Trigger_2024`

…t match

Additional tracks filter for vertices (minimum no. strip hits)

Fix Alpaka Harvesting for Phase2

Reset Pixel Dead channels map Online GUI

Phase-2 GT interface update and slice test pattern writers

…g-format [ALCA-DB-L1] Changes suggested by new llvm18 clang-format

Fix mismatch between EB and EE recHits in Phase2 supercluster producer configs

…atch Better exception message if module label in ESInputTag doesn't match

…ormat [ALCA-L1] Changes suggested by new llvm18 clang-format

o2oRun_SiStripDCS email warning fix

Add 2024 HI eras to reco processing configuation

…TCore_realfiles_batch6 Batch 6 for updates to LST integration in cms-sw

…ecks/cms-sw-PR-45117/41778/code-format.patch; the result looks less readable, but we have to comply

…alfiles

ariostas force-pushed the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles branch 2 times, most recently from f173c6b to 8ced0f1 Compare May 29, 2024 18:41

VourMa reviewed May 30, 2024

View reviewed changes

VourMa reviewed May 31, 2024

View reviewed changes

RecoTracker/LSTCore/standalone/setup.sh Outdated Show resolved Hide resolved

slava77 reviewed May 31, 2024

View reviewed changes

ariostas force-pushed the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles branch from 83cc530 to 891eb11 Compare May 31, 2024 18:41

ariostas force-pushed the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles branch from dc4e61f to 7c6bcfa Compare June 5, 2024 18:52

VourMa mentioned this pull request Jun 8, 2024

Changes to make standalone run also from within a CMSSW area #36

Merged

slava77 mentioned this pull request Jun 26, 2024

Update pixel maps to D110 geometry #51

Open

VourMa reviewed Jun 28, 2024

View reviewed changes

RecoTracker/LSTCore/interface/alpaka/Constants.h Outdated Show resolved Hide resolved

VourMa reviewed Jun 28, 2024

View reviewed changes

RecoTracker/LSTCore/src/alpaka/Quintuplet.h Outdated Show resolved Hide resolved

VourMa reviewed Jun 28, 2024

View reviewed changes

RecoTracker/LSTCore/src/alpaka/Quintuplet.h Outdated Show resolved Hide resolved

cmsbuild and others added 30 commits September 10, 2024 09:18

Merge pull request cms-sw#45954 from mmusich/fix_45952

a05eb1b

fix `SiStripBadStrip_PayloadInspector` after merging of cms-sw#45795

Merge pull request cms-sw#45745 from Dr15Jones/modernizeException

9ff7737

Updated cms::Exception using modern C++

Merge pull request cms-sw#45916 from smuzaffar/dqm-llvm18-clang-format

1c79bd4

[DQM] Changes suggested by new llvm18 clang-format

run_class_checker.sh: parse build logs for warnings/errors

78b3f74

filter tracks for vertices with minStrip hits

f368290

Merge pull request cms-sw#45926 from smuzaffar/alca-upgrade-llvm18-cl…

2e21c11

…ang-format [ALCA-UPGRADE] Changes suggested by new llvm18 clang-format

Merge pull request cms-sw#45932 from pallabidas/Phase-2_GCT_HF_updates

a377ccd

[Phase 2 L1T] fixing ASAN errors in Phase2L1CaloJetEmulator

Merge pull request cms-sw#45951 from bsunanda/Phase2-hgx360A

21de629

Phase2-hgx360A Modify the V19 longitudinal structure of HGCal according to Meeting specification of 06/09/2024

Merge pull request cms-sw#45713 from missirol/devel_renameL1T2024Mod

dbc40d6

rename modifier `run3_2024_L1T` to `stage2L1Trigger_2024`

Add reason to exception message if module label in ESInputTag does no…

35ea795

…t match

Merge pull request cms-sw#45968 from theochatzis/addStripHitsCut_14_2_X

631818a

Additional tracks filter for vertices (minimum no. strip hits)

actually commit changes

c35176a

Merge pull request cms-sw#45963 from AdrianoDee/fix_alpaka_harvesting

0fac3f2

Fix Alpaka Harvesting for Phase2

Merge pull request cms-sw#45955 from alaperto/reset_Dead_channels_map

20dc5ff

Reset Pixel Dead channels map Online GUI

Merge pull request cms-sw#45606 from p2l1-gtEmulator/pattern-writing

58c198d

Phase-2 GT interface update and slice test pattern writers

o2oRun_SiStripDCS email warning fix

7c0c306

Update reco processing config with 2024 HI eras

f7ea571

add new files

cbbc24f

Merge pull request cms-sw#45919 from smuzaffar/alca-db-l1-llvm18-clan…

865504b

…g-format [ALCA-DB-L1] Changes suggested by new llvm18 clang-format

Merge pull request cms-sw#45973 from RSalvatico/fixEBEE

195c3b8

Fix mismatch between EB and EE recHits in Phase2 supercluster producer configs

Merge pull request cms-sw#45947 from wddgit/eventSetupModuleLabelMism…

8f0aba6

…atch Better exception message if module label in ESInputTag doesn't match

Merge pull request cms-sw#45927 from smuzaffar/alca-l1-llvm18-clang-f…

452f428

…ormat [ALCA-L1] Changes suggested by new llvm18 clang-format

added new era to run_CfgTest_5.sh

ff2d67d

add 2024 pp ref

c12ee11

Merge pull request cms-sw#45977 from JanChyczynski/SiStrip_O2O_mail_fix

242094f

o2oRun_SiStripDCS email warning fix

Merge pull request cms-sw#45981 from mandrenguyen/hi2024T0Scenario

c312b85

Add 2024 HI eras to reco processing configuation

Merge pull request #88 from SegmentLinking/CMSSW_14_1_0_pre3_LST_X_LS…

2bc333b

…TCore_realfiles_batch6 Batch 6 for updates to LST integration in cms-sw

new style code-checks from cms-bot https://cmssdt.cern.ch/SDT/code-ch…

5f9c2f6

…ecks/cms-sw-PR-45117/41778/code-format.patch; the result looks less readable, but we have to comply

Merge tag 'CMSSW_14_2_0_pre1' into CMSSW_14_1_0_pre3_LST_X_LSTCore_re…

135e18b

…alfiles

Updated setup script to CMSSW_14_2_0_pre1

83897c8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LST and LSTCore #30

Add LST and LSTCore #30

ariostas commented May 29, 2024

ariostas commented May 29, 2024

VourMa commented May 29, 2024

slava77 commented May 29, 2024

ariostas commented May 29, 2024

VourMa left a comment

VourMa May 30, 2024

slava77 May 30, 2024

ariostas May 31, 2024

VourMa commented May 30, 2024 •

edited

Loading

slava77 commented May 30, 2024

slava77 commented May 30, 2024

VourMa commented May 31, 2024 •

edited

Loading

slava77 May 29, 2024

slava77 May 29, 2024

VourMa Jun 7, 2024

slava77 May 30, 2024

slava77 May 30, 2024

VourMa Jun 12, 2024

ariostas commented May 31, 2024

ariostas commented May 31, 2024

VourMa commented May 31, 2024

ariostas commented May 31, 2024

ariostas commented May 31, 2024

slava77 commented May 31, 2024

ariostas commented Jun 13, 2024

VourMa commented Jun 13, 2024

slava77 commented Jun 13, 2024

VourMa commented Jun 28, 2024

	#define Constants_cuh
	#define RecoTracker_LSTCore_alpaka_Constants_cuh

Add LST and LSTCore #30

Are you sure you want to change the base?

Add LST and LSTCore #30

Conversation

ariostas commented May 29, 2024

ariostas commented May 29, 2024

VourMa commented May 29, 2024

slava77 commented May 29, 2024

ariostas commented May 29, 2024

VourMa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VourMa commented May 30, 2024 • edited Loading

slava77 commented May 30, 2024

slava77 commented May 30, 2024

VourMa commented May 31, 2024 • edited Loading

UPDATE:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ariostas commented May 31, 2024

ariostas commented May 31, 2024

VourMa commented May 31, 2024

ariostas commented May 31, 2024

ariostas commented May 31, 2024

slava77 commented May 31, 2024

ariostas commented Jun 13, 2024

VourMa commented Jun 13, 2024

slava77 commented Jun 13, 2024

VourMa commented Jun 28, 2024

VourMa commented May 30, 2024 •

edited

Loading

VourMa commented May 31, 2024 •

edited

Loading