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

Switching off reconstruction of so called "NullTaus" #17767

Merged

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented Mar 3, 2017

Hi All,

it is a copy of #17706 rebased to the main branch (91X). Description of the original RP cited below for reference. The #17706 will be closed in a minute.

Best,
Michał

Hi all,

this is a simple configuration switch to remove the so called "null-taus" from the tau reconstruction. Null taus are jets seeding tau candidates, which actually do not fulfill any good tau reconstruction criterion. In the past these had been kept in the reconstruction for debugging purposes, since it allows a 1:1 correspondence of reconstructed tau candidates with seeding jets. In the meantime this should be switched to False when running in standard reconstruction. For more details refer to @mbluj.

NB:
we have split this change off from #17620 (90X) for clarity reasons. See also discussion in the conversation there.

Cheers,
Roger

Add an additional requirement that PFChargedHardon exists. It protects against poor taus build around RecoTauChargedHadron candidates with a poor track which usually have null or very small momentum.
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

A new Pull Request was created by @mbluj for master.

It involves the following packages:

RecoTauTag/RecoTau

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@slava77
Copy link
Contributor

slava77 commented Mar 3, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18132/console Started: 2017/03/03 18:52

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

@davidlange6 davidlange6 merged commit e8cf191 into cms-sw:master Mar 13, 2017
@slava77
Copy link
Contributor

slava77 commented Mar 14, 2017

@davidlange6
why was this merged without a reco signature?

@davidlange6
Copy link
Contributor

err- thats a good question. I think I clicked on the wrong PR. Shall i unmerge it?

@slava77
Copy link
Contributor

slava77 commented Mar 14, 2017

@davidlange6
I'll check it today and will let you know if it should be reverted.

@slava77
Copy link
Contributor

slava77 commented Mar 14, 2017

@mbluj @roger-wolf
in quite a few workflow comparisons there are differences in DQM like this (from wf 136.731, SinglePh2016B data)
wf136 731_pftau_loosemu3_sumpt

Are these plots missing a basic tau quality preselection or did this update remove more than necessary?

@mbluj
Copy link
Contributor Author

mbluj commented Mar 14, 2017

Yes, this plot is done without checking if a tau candidate passes basic quality criteria as presence of leading track. Differences are expected: it should be less tau candidates with low values of isolation sum.

@slava77
Copy link
Contributor

slava77 commented Mar 14, 2017 via email

@mbluj
Copy link
Contributor Author

mbluj commented Mar 14, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Mar 15, 2017

+1

for #17767 b09eb1b

  • post-merge "+1"
  • changes are as described, affecting tau reco configuration
  • jenkins tests pass and comparisons with baseline show differences in tau objects
  • local test with modified validate.C shows that differences in recoPFTaus_hpsPFTauProducer go away after a requirement that leadPFChargedHadrCandsignedSipt_ is non-NaN, the differences in discriminants go away after requiring the discriminant values (non-binary cases) are non-zero
  • either jenkins or local tests show no differences in miniAOD tau objects, confirming that after basic selections there are no changes as expected.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants