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

PF-based b-taggers in the standard Reco #7041

Conversation

ferencek
Copy link
Contributor

@ferencek ferencek commented Jan 5, 2015

This PR is the first step toward replacing the legacy with PF-based b-taggers. Here, the PF-based b-taggers are added to the standard Reco sequence alongside the legacy b-taggers. In subsequent PRs validation, PAT, etc. need to be switched to the PF-based taggers and finally the legacy taggers will be removed from the standard Reco sequence.

In the attached plots and in https://indico.cern.ch/event/359461/session/0/contribution/2/material/slides/0.pdf performance comparison between the legacy and PF-based taggers can be seen.

Legacy vs PF-based taggers from MiniAOD

btageffvsmistagrate_udsg_pf
btageffvsmistagrate_c_pf

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 5, 2015

A new Pull Request was created by @ferencek (Dinko Ferencek) for CMSSW_7_4_X.

PF-based b-taggers in the standard Reco

It involves the following packages:

FastSimulation/ParticleFlow
PhysicsTools/PatAlgos
RecoBTag/Configuration
RecoBTag/ImpactParameter
RecoBTag/SecondaryVertex
RecoBTau/JetTagComputer
RecoParticleFlow/Configuration

@civanch, @nclopezo, @lveldere, @mdhildreth, @monttj, @cmsbuild, @StoyanStoynev, @slava77, @vadler can you please review it and eventually sign? Thanks.
@TaiSakuma, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @lgray, @Martin-Grunewald, @nhanvtran, @schoef, @ferencek, @matt-komm, @mariadalfonso, @pvmulder, @bachtis this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@ferencek
Copy link
Contributor Author

ferencek commented Jan 7, 2015

@cmsbuild please test

@slava77
Copy link
Contributor

slava77 commented Jan 8, 2015

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 8, 2015

The tests are being triggered in jenkins.

@@ -0,0 +1,6 @@
import FWCore.ParameterSet.Config as cms

pfNegativeOnlyJetProbabilityBJetTags = cms.EDProducer("JetTagProducer",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's particularly practical to define so many almost identical cfi files.
If you decide to add some common parameters to JetTagProducer, the maintenance will be more costly than having one default and cloning with replace from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the JetTagProducers cloning would not be that useful since all parameters are typically updated. So these I won't modify.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 8, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 9, 2015

@slava77
Copy link
Contributor

slava77 commented Jan 9, 2015

+1

for #7041 1cd5296

tested in CMSSW_7_4_X_2015-01-07-1000 (test area sign484)

new products appear as expected from

  • pfInclusiveSecondaryVertexFinderTagInfos
  • inclusiveCandidateSecondaryVertices
  • pfCombinedInclusiveSecondaryVertexV2BJetTags
  • pfJetBProbabilityBJetTags
  • pfCombinedMVABJetTags
  • pfJetProbabilityBJetTags
  • pfTrackCountingHighEffBJetTags
  • pfTrackCountingHighPurBJetTags
  • pfSimpleSecondaryVertexHighEffBJetTags
  • pfSimpleSecondaryVertexHighPurBJetTags

the cost for AOD (ttbar PU35@25ns) is ~ 0.2%;
CPU additions are driven by the inclusive vertexing with candidates and takes 0.4% of RECO, which is a bit too much.
Since this is temporary, it's OK for now.

@slava77
Copy link
Contributor

slava77 commented Jan 9, 2015

@monttj
TaeJeong, please check, we need your signature here.
Thank you.

davidlange6 added a commit that referenced this pull request Jan 11, 2015
…_7_4_X_2015-01-04-1400

PF-based b-taggers in the standard Reco
@davidlange6 davidlange6 merged commit 941af5c into cms-sw:CMSSW_7_4_X Jan 11, 2015
@ferencek ferencek deleted the ParticleFlowBTag_from_CMSSW_7_4_X_2015-01-04-1400 branch January 11, 2015 23:03
@monttj
Copy link
Contributor

monttj commented Jan 12, 2015

+1
I see it is already merged. I am sorry for being late.

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

5 participants