CA - Last hit quadruplet chi2 filter #17083
Conversation
|
A new Pull Request was created by @felicepantaleo (Felice Pantaleo) for CMSSW_9_0_X. It involves the following packages: RecoPixelVertexing/PixelTriplets @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
@mtosi @JanFSchulte |
|
@cmsbuild , please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
| isTheSameTriplet = (quadId != 0) && (foundQuadruplets[quadId][0]->getCellId() == previousCellIds[0]) && (foundQuadruplets[quadId][1]->getCellId() == previousCellIds[1]); | ||
| isTheSameFourthLayer = (quadId != 0) && (fourthLayerId == previousfourthLayerId) && (subDetId == previousSubDetId) && (sideId == previousSideId); | ||
|
|
||
| previousCellIds = {{foundQuadruplets[quadId][0]->getCellId(), foundQuadruplets[quadId][1]->getCellId()}}; |
There was a problem hiding this comment.
some reduction of copy-paste, especially inplace multi-operator calls would be useful
|
+1
Based on the supplied MTV plots, the effect is pretty clear at the seeding level.
|
cmsbuild
added
fully-signed
and removed
pending-signatures
reconstruction-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar |
|
why don't we want to have the OnlyOneLastHitPerLayerFilter set to True as default ? |
|
@mtosi it's false if you don't specify that you want to use it in the configuration. |
|
@mtosi Setting it optional now was my request for mostly to be cautious (even if in general I'm in favour of it). The PR adding workflows for CA in offline (#16911) indicated a problem in b tagging that we are currently investigating. I'd prefer to not change the offline-default CA parameters until the origin of the problem is fully understood (or it is demonstrated that the change cures the problem). I mainly feel that in addition to Felice's tests the feature would benefit from some further studies (for both offline and HLT) before switching it on. Integrating the feature as default-off allows Felice/us to integrate it now without affecting anything. OTOH, if HLT prefers to switch it on by default, that's fine for me (as long as it is only for HLT for now). |
|
I see, thanks !
mia
…On Wed, Dec 21, 2016 at 10:53 AM, Matti Kortelainen < ***@***.***> wrote:
@mtosi <https://github.com/mtosi> Setting it optional now was my request
for mostly to be cautious (even if in general I'm in favour of it).
The PR adding workflows for CA in offline (#16911
<#16911>) indicated a problem in b
tagging that we are currently investigating. I'd prefer to not change the
offline-default CA parameters until the origin of the problem is fully
understood (or it is demonstrated that the change cures the problem).
I mainly feel that in addition to Felice's tests the feature would benefit
from some further studies (for both offline and HLT) before switching it
on. Integrating the feature as default-off allows Felice/us to integrate it
now without affecting anything. OTOH, if HLT prefers to switch it on by
default, that's fine for me (as long as it is only for HLT for now).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17083 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEt583HHVJKpJC4NbKNLa8BJ0-PYETBzks5rKPcggaJpZM4LR-E_>
.
|
|
+1 |
Adding the possibility to have a filter on the last hit of a quadruplet, like in quadruplets by propagation.
If two quadruplets have the first three hits in common and the fourth hits are on the same layer, pick the one with the lower chi2.
This has the effect of reducing the fake and duplicate rate within the same iteration: e.g. https://fpantale.web.cern.ch/fpantale/offline_tuning/lastLayerChi2FilterPR/plots_initialStepPreSplitting.html
blue Quadruplets by propagation
red CA in the release
black CA with filter turned on (this PR)
@VinInn @rovere @makortel