Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

VourMa
Copy link
Collaborator

@VourMa VourMa commented May 17, 2024

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 #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
Copy link

slava77 commented May 17, 2024

@VourMa
the changes look OK to me.

I started tests in
SegmentLinking/TrackLooper#403

@slava77
Copy link

slava77 commented May 17, 2024

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
Copy link

slava77 commented May 17, 2024

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
Copy link
Collaborator Author

VourMa commented May 19, 2024

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
Copy link

slava77 commented May 19, 2024

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
Copy link
Collaborator Author

VourMa commented May 19, 2024

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
Copy link
Collaborator Author

VourMa commented May 20, 2024

Superseded by #28

@VourMa VourMa closed this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants