search

SegmentLinking / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1 stars 1 forks source link

Usage of split TrackCandidate collections for LST objects and fixes/improvements for TrajectorySeed collections #27

Closed VourMa closed 1 month ago

VourMa commented 1 month ago

Same as #25 but with 14_1_0_pre3 as target branch. Copying a summary of the description here (and more detailed comments can be found in the other PR):

This PR transfers the machinery for the improvements introduced in Phase 2 HLT for LST and partially transfers these improvements. More specifically, the pT LST objects and the T5 LST objects are now used to create separate TrackCandidate collections, so that a different tracking ID can be applied to each one of them: The default tracking ID for highPtTripletStep for Phase 2 is applied to the pT LST objects and not tracking ID is applied to the T5 LST objects. What can also be tested and applied in a future PR is the utilization of the LST seeds as inputs to a following CKF/mkFit "iteration".

Apart from the above, there are a few fixes/improvements for the TrajectorySeeds collection of LST objects (see PR https://github.com/SegmentLinking/cmssw/pull/24 and the discussion that took place there about the proper length for the hitsForSeed vector and the addition of a toggle to construct non-pLS TrajectorySeeds).

The plots for the two-iteration setup with CKF vs. LST vs. LST after this PR can be found here: https://uaf-10.t2.ucsd.edu/~evourlio/SDL/CMSSWPR25/CKF_LST_LSTAfterAllUpdates/

slava77 commented 1 month ago

@VourMa the changes look OK to me.

I started tests in https://github.com/SegmentLinking/TrackLooper/pull/403

slava77 commented 1 month ago

I started tests in SegmentLinking/TrackLooper#403

ah, I missed that this was made for the LSTCore branch. So, the CI would not help here; at least not yet

slava77 commented 1 month ago

I'd rather the LSTCORE branch does not diverge in functionality from the regular LST_X branch, at least until our CI moves to it by default.

VourMa commented 1 month ago

I'd rather the LSTCORE branch does not diverge in functionality from the regular LST_X branch, at least until our CI moves to it by default.

Sure, would you like me to make the same PR in the non-LSTCore branch? I do not see any dependence there, so it should be straightforward.

slava77 commented 1 month ago

Sure, would you like me to make the same PR in the non-LSTCore branch? I do not see any dependence there, so it should be straightforward.

this would work. Thanks. Note that you can Edit the PR and change the target branch

VourMa commented 1 month ago

Note that you can Edit the PR and change the target branch

This would push also the LSTCore package to the default branch, which better be done in a separate PR. I will just prepare a proper branch with only the updates needed, and have it ready for merging before our meeting tomorrow.

VourMa commented 1 month ago

Superseded by #28