Logic bug in CutBasedElectronID

[Dr15Jones]

The line

cmssw/RecoEgamma/ElectronIdentification/plugins/CutBasedElectronID.cc

Line 145 in f912613

if (version_ != "V01" or version_ != "V00") {

is faulty as the statement will always return true. What was probably intended was

if (version_ != "V01" and version_ != "V00") {

[Dr15Jones]

assign RecEgamma/ElectronIdentification

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @Dr15Jones.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[makortel]

assign RecoEgamma/ElectronIdentification

[cmsbuild]

New categories assigned: reconstruction

@jfernan2,@mandrenguyen you have been requested to review this Pull request/Issue and eventually sign? Thanks

[Dr15Jones]

The bug was introduced 14 years ago during a fix of a different logical bug

cms-cvs-history/RecoEgamma-ElectronIdentification@279b68e

[Dr15Jones]

looks like version 'V05' was the largest version number 14 years ago. Given the 'default' appears to be an empty version, which then defaults to the behavior matching the largest known version, it shows why the bug associated to version 'V00' and 'V01' was not noticed.

[jfernan2]

@SanghyunKo @cochando as L3 Reco EGamma conveners, can you please comment on the solution? Thanks

[Dr15Jones]

So the affect of the bug is:

The value of ip is irrelevant as that variable is never used in the V00 and V01 code.

The value of sigmaee for the case where electron->isEB() is true and version is V00 was supposed to be

sigmaee = electron->sigmaEtaEta();

while for V01 it was supposed to be

double sigmaee = electron->sigmaIetaIeta();

while the bug makes them both

sigmaee = electron->sigmaIetaIeta();

which is fine for V01 but wrong for V00.

[SanghyunKo]

From what I've seen briefly, this module and the similar one mentioned in issue #46868 seem to be outdated; as far as I can tell, these are not the modules used for the current Run2 & Run3 cut-based EGM ID.

I'll trace back to figure out whether we have any customers (other than the JME module mentioned in #46868).