Fix inconsistency in soft lepton taginfos naming #11247
Conversation
…ntually in separate PR
|
A new Pull Request was created by @imarches for CMSSW_7_6_X. Fix inconsistency in soft lepton taginfos naming It involves the following packages: DataFormats/BTauReco @cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks. |
|
@imarches, can you also update |
| @@ -109,7 +113,7 @@ void SoftPFElectronTagInfoProducer::produce(edm::Event& iEvent, const edm::Event | |||
| properties.ratioRel = recoelectron->p4().Dot(jetRef->p4()) / pjet.Mag2(); | |||
| properties.p0Par = boostedPPar(recoelectron->momentum(), jetRef->momentum()); | |||
| properties.elec_mva = recoelectron->mva_e_pi(); | |||
| if(std::abs(properties.sip3d>MaxSip3D)) continue; | |||
| if(std::abs(properties.sip3dsig>MaxSip3Dsig)) continue; | |||
There was a problem hiding this comment.
Good catch! I guess the original intention was to have std::abs(properties.sip3dsig)>MaxSip3Dsig. In practice I suspect the impact of this bug is negligible but still it should be fixed. Thanks again for catching it.
There was a problem hiding this comment.
Yes, I'll add a commit to fix it. Yes, the impact should be negligible, as the cut is set by default at 200, that's why it passed unnoticed until now.
|
Hallo @ferencek yes I can also add the changes to the DQM to this same PR. I'll work at it this afternoon. |
|
should we expect #11247 (comment) to be addressed as well? |
|
Hello @slava77 yes, please, wait a moment for the DQM things, I will address them right away here, as I'm about it anyway. Thanks! I will make the commit in a while. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
@imarches: |
|
+1 Fixing a bug in SoftPFElectronTagInfoProducer.cc and renaming of signed IP significance variables used for soft lepton tag infos. The code changes are satisfactory, and Jenkins tests against baseline CMSSW_7_6_X_2015-09-13-1100 show no significant differences. The bug fix makes tiny changes in a few quantities related to the combined MVA and soft PF electron b taggers. |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
cmsbuild
added
reconstruction-approved
fully-signed
and removed
pending-signatures
reconstruction-pending
labels
|
@cvuosalo I have added this explanation to the description. Thanks |
|
+1 |
…or76X Fix inconsistency in soft lepton taginfos naming
Fixing an inconsistency in the soft lepton taginfos naming, dating back to 2007:
cms-cvs-history/DataFormats-BTauReco@1a96735#diff-e1aaed9bb312dbfd1dee141f852b4e78
Due to this inconsistency, the quantities:
sip2d, sip3d
are used as IP significances, while they are meant to be the signed IP values. These quantities are filled now with the IP values and two new taginfo variables are added for the significances themselves:
sip2dsig, sip3dsig
This fix solves an error-prone naming ambiguity.
This PR fixes also a bug in the use of brackets in SoftPFElectronTagInfoProducer.cc, where the original:
if(std::abs(properties.sip3d>MaxSip3D)) continue
was actually meant to be:
if(std::abs(properties.sip3d)>MaxSip3D) continue
This fix produces small differences with respect to the original version of the code, though not significantly affecting the performance.