Changed logic of dZVertex assignement #18473
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A new Pull Request was created by @arizzi for master. It involves the following packages: CommonTools/RecoAlgos @perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
arizzi
commented
Apr 26, 2017
| maxDzSigForPrimaryAssignment = cms.double(5.0), # in OR with next | ||
| maxDzForPrimaryAssignment = cms.double(0.03), # in OR with prev | ||
| maxDzSigForPrimaryAssignment = cms.double(5.0), # in AND with next | ||
| maxDzForPrimaryAssignment = cms.double(0.03), # in AND with prev |
There was a problem hiding this comment.
this should be moved to 0.1 too
What physics observable plots can show that this change is an improvement overall (or is harmless for most analyses)? Thank you. |
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
Pull request #18473 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again. |
|
@slava77 studies are still ongoing. This should improve performance also at 200PU (in fact it should be a larger improvemente there). |
|
@cmsbuild please test |
|
On 5/4/17 10:51 AM, arizzi wrote:
I do not undersand the point, if a track claim to be measured with 50um
dZ accuracy and is not used in fit and is not compatible with the PV
within 5sigma... why should I consider it as surely part of the vertex?
...5 sigma is pretty large...
I'm not after the dzError of 50 um (a good bulk of resolution), but
after much smaller values which are likely under-measurements.
Can we trust dzError = 1 um and throw away the track because it is 5
um away from a vertex?
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18473 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbmXT9LzNeiksa60SYWqgyM8jra2vks5r2hAwgaJpZM4NIpIL>.
|
|
uncertainty in physics are as important as the measured values. I expect
that no track claims a 1um dzError. If it does, that's clearly a bug to
report to TRK POG.
On Thu, May 4, 2017 at 7:58 PM, Slava Krutelyov <notifications@github.com>
wrote:
… On 5/4/17 10:51 AM, arizzi wrote:
> I do not undersand the point, if a track claim to be measured with 50um
> dZ accuracy and is not used in fit and is not compatible with the PV
> within 5sigma... why should I consider it as surely part of the vertex?
> ...5 sigma is pretty large...
I'm not after the dzError of 50 um (a good bulk of resolution), but
after much smaller values which are likely under-measurements.
Can we trust dzError = 1 um and throw away the track because it is 5
um away from a vertex?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#18473 (comment)>, or
> mute the thread
> <https://github.com/notifications/unsubscribe-auth/AEdcbmXT9
LzNeiksa60SYWqgyM8jra2vks5r2hAwgaJpZM4NIpIL>.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18473 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEyiljSd4Ql9P9FZRdFfJZBrEBmY7CnNks5r2hHIgaJpZM4NIpIL>
.
|
|
On 5/4/17 1:12 PM, arizzi wrote:
uncertainty in physics are as important as the measured values.
I expect
that no track claims a 1um dzError. If it does, that's clearly a bug to
report to TRK POG.
OK, trying to get a similar result with somewhat different arguments:
It's important to account for uncertainty on the distance when applying
a selection on the distance.
Shouldn't the uncertainty of the vertex position be considered in the dz
selection?
Here is an example with some numbers, looking at a phase-2 file:
values of track dzError in the range of 9um to 10 um are frequent (mpv
is 14 for muons pt>~20)
We are talking here about tracks that were not used in the vertex fit
(e.g. the rest of the vertex was split off ).
An average leading PV has zError of 7 um in ttbar events.
A median non-leading PV has zError of ~35 um.
So, for this example track with dzError of 10 um, the distance selection
to a vertex of dz<5*dzError becomes dz < 50um, which is just above 1
sigma for non-leading PV and looks too tight.
Wouldn't it make sense in this case to use the uncertainty on the
distance to the vertex?
…
On Thu, May 4, 2017 at 7:58 PM, Slava Krutelyov ***@***.***>
wrote:
> On 5/4/17 10:51 AM, arizzi wrote:
> > I do not undersand the point, if a track claim to be measured with 50um
> > dZ accuracy and is not used in fit and is not compatible with the PV
> > within 5sigma... why should I consider it as surely part of the vertex?
> > ...5 sigma is pretty large...
>
> I'm not after the dzError of 50 um (a good bulk of resolution), but
> after much smaller values which are likely under-measurements.
>
> Can we trust dzError = 1 um and throw away the track because it is 5
> um away from a vertex?
>
>
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub
> > <#18473 (comment)>, or
> > mute the thread
> > <https://github.com/notifications/unsubscribe-auth/AEdcbmXT9
> LzNeiksa60SYWqgyM8jra2vks5r2hAwgaJpZM4NIpIL>.
>
> >
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#18473 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AEyiljSd4Ql9P9FZRdFfJZBrEBmY7CnNks5r2hHIgaJpZM4NIpIL>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18473 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbqpzEDaN5feNlmDkCc72q6Po6Vc8ks5r2jFEgaJpZM4NIpIL>.
|
|
The solution is to add the dZerror of the vertex in quadrature with the
dzerror of the track. Not to escape the sigma cuts.
Il 04 mag 2017 23:16, "Slava Krutelyov" <notifications@github.com> ha
scritto:
… On 5/4/17 1:12 PM, arizzi wrote:
> uncertainty in physics are as important as the measured values.
> I expect
> that no track claims a 1um dzError. If it does, that's clearly a bug to
> report to TRK POG.
OK, trying to get a similar result with somewhat different arguments:
It's important to account for uncertainty on the distance when applying
a selection on the distance.
Shouldn't the uncertainty of the vertex position be considered in the dz
selection?
Here is an example with some numbers, looking at a phase-2 file:
values of track dzError in the range of 9um to 10 um are frequent (mpv
is 14 for muons pt>~20)
We are talking here about tracks that were not used in the vertex fit
(e.g. the rest of the vertex was split off ).
An average leading PV has zError of 7 um in ttbar events.
A median non-leading PV has zError of ~35 um.
So, for this example track with dzError of 10 um, the distance selection
to a vertex of dz<5*dzError becomes dz < 50um, which is just above 1
sigma for non-leading PV and looks too tight.
Wouldn't it make sense in this case to use the uncertainty on the
distance to the vertex?
>
>
>
> On Thu, May 4, 2017 at 7:58 PM, Slava Krutelyov <
***@***.***>
> wrote:
>
>> On 5/4/17 10:51 AM, arizzi wrote:
>> > I do not undersand the point, if a track claim to be measured with
50um
>> > dZ accuracy and is not used in fit and is not compatible with the PV
>> > within 5sigma... why should I consider it as surely part of the
vertex?
>> > ...5 sigma is pretty large...
>>
>> I'm not after the dzError of 50 um (a good bulk of resolution), but
>> after much smaller values which are likely under-measurements.
>>
>> Can we trust dzError = 1 um and throw away the track because it is 5
>> um away from a vertex?
>>
>>
>> >
>> > —
>> > You are receiving this because you were mentioned.
>> > Reply to this email directly, view it on GitHub
>> > <#18473 (comment)>,
or
>> > mute the thread
>> > <https://github.com/notifications/unsubscribe-auth/AEdcbmXT9
>> LzNeiksa60SYWqgyM8jra2vks5r2hAwgaJpZM4NIpIL>.
>>
>> >
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> <#18473 (comment)>,
or mute
>> the thread
>> <https://github.com/notifications/unsubscribe-auth/
AEyiljSd4Ql9P9FZRdFfJZBrEBmY7CnNks5r2hHIgaJpZM4NIpIL>
>> .
>>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#18473 (comment)>, or
> mute the thread
> <https://github.com/notifications/unsubscribe-auth/
AEdcbqpzEDaN5feNlmDkCc72q6Po6Vc8ks5r2jFEgaJpZM4NIpIL>.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18473 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEyiljz24cuoJRVqVceshf1_wB5Gol98ks5r2j_mgaJpZM4NIpIL>
.
|
|
On 5/4/17 9:52 PM, arizzi wrote:
The solution is to add the dZerror of the vertex in quadrature with the
dzerror of the track. Not to escape the sigma cuts.
Right,
that was my second more meaningful iteration of comments.
Can this happen in this PR?
Please clarify.
Thank you.
…
Il 04 mag 2017 23:16, "Slava Krutelyov" ***@***.***> ha
scritto:
> On 5/4/17 1:12 PM, arizzi wrote:
> > uncertainty in physics are as important as the measured values.
>
> > I expect
> > that no track claims a 1um dzError. If it does, that's clearly a bug to
> > report to TRK POG.
>
> OK, trying to get a similar result with somewhat different arguments:
> It's important to account for uncertainty on the distance when applying
> a selection on the distance.
> Shouldn't the uncertainty of the vertex position be considered in the dz
> selection?
>
> Here is an example with some numbers, looking at a phase-2 file:
> values of track dzError in the range of 9um to 10 um are frequent (mpv
> is 14 for muons pt>~20)
> We are talking here about tracks that were not used in the vertex fit
> (e.g. the rest of the vertex was split off ).
> An average leading PV has zError of 7 um in ttbar events.
> A median non-leading PV has zError of ~35 um.
> So, for this example track with dzError of 10 um, the distance selection
> to a vertex of dz<5*dzError becomes dz < 50um, which is just above 1
> sigma for non-leading PV and looks too tight.
>
>
> Wouldn't it make sense in this case to use the uncertainty on the
> distance to the vertex?
>
|
|
sure, but do not expect another round of validation on many samples. |
|
On 5/4/17 11:29 PM, arizzi wrote:
sure, but do not expect another round of validation on many samples.
It would be nice still to check on the current most discrepant few
samples (both in absolute and significance terms).
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18473 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbteOmFai1X0LpGfzOCpECSww50Sbks5r2sHFgaJpZM4NIpIL>.
|
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
I've added columns in the spreadsheet at I can run on some more selected samples among those in the list if someone request for checks on particular samples |
|
Comparison is ready Comparison Summary:
|
|
On 5/5/17 6:38 AM, arizzi wrote:
I've added columns in the spreadsheet at
https://docs.google.com/spreadsheets/d/1Qbrcdya4NNhOyKFebg-EfkUEn8DxwiOSFXZXwnw2Rw8/edit#gid=1071349842
for 4 samples.
Changes are mostly non significant and anyhow positive, the only
significant one is positive.
Thank you.
These should be enough.
… I can run on some more selected samples among those in the list if
someone request for checks on particular samples
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18473 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbotS1x-pCny2soMd345hyB3m-JgLks5r2yZWgaJpZM4NIpIL>.
|
|
+1
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The category PrimaryDz (CompatibilityDz in MiniAOD) is using the OR between compatibility in value and significance.
This PR changes the OR to an AND in order to avoid outliers.
Because now it is an "AND" the absolute value cut is also increased from 300um to 1mm
This is a request received long ago from VBF community and recently reported to create troubles on searches on gamma+jets samples.
@azzurrip @gpetruc @rovere @VinInn
PS: basic tests are done but we should check on larger samples before merging