updates to handle pLS with OT hits#253
Conversation
|
run-ci: [all, hlt] |
|
run-ci: [all, hlt] |
@ariostas is it from me incorrectly formatting the command or something else; the error sounds like something else |
|
The PR was built and ran successfully in standalone mode running on GPU. Here are some of the comparison plots.
The full set of validation and comparison plots can be found here. Here is a timing comparison: |
|
run-ci: hlt |
|
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: |
|
The PR was built and ran successfully with CMSSW running on GPU. Here are some plots. OOTB All Tracks
The full set of validation and comparison plots can be found here. |
|
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. |
| bool pmatched = false; | ||
| if (phits1[i] == -1) | ||
| // short pLSs are padded with the last hit idx | ||
| if (i + 1 == Params_pLS::kHits && phits1[i] == phits1[i - 1]) |
There was a problem hiding this comment.
note to self:
here the hits are not in the same order. So, 3-hit pLS will have the same hit at i=1 and i=3.
OTOH, hit indices are always filled with something non-default. So, this check here if corrected is only to save a cycle of checks with j
55057df to
565d599
Compare
|
run-ci: [all, hlt] |
|
There was a problem while building and running in standalone mode. The logs can be found here. |
|
There was a problem while building and running with CMSSW. The logs can be found here. |
565d599 to
0c2097e
Compare
|
run-ci: [all, hlt] |
|
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: |
|
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. |
|
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. |
|
run-ci: [all, hlt] |
|
The PR was built and ran successfully in standalone mode running on GPU. Here are some of the comparison plots.
The full set of validation and comparison plots can be found here. Here is a timing comparison: |
|
run-ci: [all, hlt] |
I expect still no changes in the offline setup but some minor increase in the fake rate in the HLT setup (based on the local tests, mostly localized in the shorter tracks) |
I had a silly typo (1 vs 2) mixup. I tried to subscribe to the pro account, but with that I was suggested to prove a negative in line with "upload an image explaining why you are not on campus" |
|
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: |
|
The PR was built and ran successfully with CMSSW running on GPU. Here are some plots. OOTB All Tracks
The full set of validation and comparison plots can be found here. |
|
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. |
|
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. |
|
The PR was built and ran successfully with HLT setup running on GPU (procModifiers = ). Here are some plots. HLT General Plots
The full set of validation and comparison plots can be found here. |
VourMa
left a comment
There was a problem hiding this comment.
The changes look good to me, I have some comments mostly for my understanding.
More in general, I am not sure what we want to do with disabling removeOTRechits. The performance looks to me worse in more or less all fronts. Is there any obvious advantage? Should this be introduced as a means to study how to improve but not enable it by default?
https://indico.cern.ch/event/1640803/#16-contribution p7, 8 may help to tilt the discussion in the direction of disabling For the fakes it looks like the first order effect for the final tracks is the |
…xtended pLS stopping in barrel and endcap OT:
- fix error computations and dBetas
- account for radius uncertainties in lumi-based and pointed z selection, most important for connections with pLS on the same tilted layer
- use the tl-axis length in dBetaRes term, it becomes large in the near-3-point cases
- rename a few more variables for clarity
- fixup to the dzDrtScale use in PPBB
- PPEE: fix dLum and dzDrtScale in rt{Lo,Hi}, fix dz sign in rt{Lo,Hi}_point
…to have a fast (and disambiguated) overlap checks; simplify pLS overlap check
…nge in pLS-LS matching
…tBits (kMaxPLSHitBitsInHitsSoA = 8) to open room for more hits
…n OT and was not extended by LST (at most 3 IT hits)
6895f88 to
2d7eb5f
Compare
|
run-ci: [all, hlt] |
|
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: |
|
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. |
|
I was able to reproduce in 17_0_0_pre2 vs 17_0_0_pre2 the change is most visible in fakes with 4 layers (3 pixel layers)
There is as much as 40% fractional decrease in the fake rate with about 1% drop in the efficiency around |
These 3IT+1OT hit fakes are also possibly migrating to 3IT+3OT. |
VourMa
left a comment
There was a problem hiding this comment.
Quick review of the logic for removing OT hits from seeds.
| const auto iSeed = lstOutput_view.pixelSeedIndex()[i]; | ||
| const bool dropHitsOTpL = | ||
| iType == lst::LSTObjType::pLS && dropOTHitsPurePLS_ && | ||
| std::prev(pixelSeeds[iSeed].recHits().end())->geographicalId().subdetId() > PixelSubdetector::PixelEndcap && |
There was a problem hiding this comment.
Are the recHits ordered from IT -> OT? Is this condition even needed?
| @@ -110,6 +116,8 @@ void LSTOutputConverter::fillDescriptions(edm::ConfigurationDescriptions& descri | |||
| desc.add<edm::InputTag>("lstPixelSeeds", edm::InputTag("lstInputProducer")); | |||
| desc.add<bool>("includeT5s", true); | |||
| desc.add<bool>("includeNonpLSTSs", false); | |||
| desc.add<bool>("dropOTHitsPurePLS", false); | |||
| desc.add<int>("maxITHitsToDropOTHitsPurePLS", 3); | |||
| for (auto const& hit : recHits) | ||
| if (hit.sharesInput(OTHits[hitIdx], TrackingRecHit::all)) { | ||
| hitOK = false; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Could you remind me what cases is this supposed to catch?
| if (dropHitsOTpL) { // true if need to drop OT hits | ||
| using Hit = SeedingHitSet::ConstRecHitPointer; | ||
| std::vector<Hit> hitsForSeed; | ||
| hitsForSeed.reserve(std::ranges::size(pixelSeeds[iSeed].recHits())); | ||
| for (auto const& hit : pixelSeeds[iSeed].recHits()) { | ||
| if (hit.geographicalId().subdetId() > PixelSubdetector::PixelEndcap) | ||
| continue; | ||
| hitsForSeed.emplace_back(dynamic_cast<Hit>(&hit)); | ||
| recHits.push_back(hit.clone()); | ||
| } | ||
| GlobalTrackingRegion region; | ||
| seedCreator_->init(region, iSetup, nullptr); | ||
| seedCreator_->makeSeed(seeds, hitsForSeed); | ||
| if (seeds.empty()) | ||
| edm::LogInfo("LSTOutputConverter") << "failed to convert a pLS object to a seed" << i << iSeed; | ||
| seed = seeds[0]; | ||
| seedRef = edm::RefToBase<TrajectorySeed>(edm::Ref(outputTSRP, outputTS.size())); | ||
| } |
There was a problem hiding this comment.
This is done only when produceSeeds_. Don't we want it also when we produce tracks?
























































runTripletDefaultAlgoPPBBandrunTripletDefaultAlgoPPEEduring pLS matching to the segments of T3. Most of the cuts needed updates: positional and directional z in the barrel (r in endcap), delta beta. The implementation is generalized from the case of pLS ending before the first segment (rightmost case) to other possibilities; near-degenerate cases are pass-through.zPointedwindow definition, where the z error already estimated at a destination was incorrectly additionally scaled bydrOutIn/dSDInis now corrected (an individual test shows minor impact on the old no-OT pLS test zPointed selection fix in PPBB #216)PixelSegmentsSoALayout::pLSHitsIdxsare now packedOT/IT<<32 + indexto have a fast (and disambiguated) overlap checksLSTOutputConverterwhen adding the OT LST TC hits after the pLS hits were addedThe initial submission does not change
removeOTRechitsto first confirm the default behavior is OK with the CI.