cmssw: Potential bug in ConversionHitChecker

Dear EGamma experts,

During my studies I’ve stumbled across the following code:

https://github.com/cms-sw/cmssw/blob/master/RecoEgamma/EgammaPhotonAlgos/src/ConversionHitChecker.cc#L17-L28

I wonder, whether there is a change necessary here:

https://github.com/cms-sw/cmssw/blob/master/RecoEgamma/EgammaPhotonAlgos/src/ConversionHitChecker.cc#L28

Shouldn’t it be trajParams[closest] instead of trajParams[0]? It seems, that in most of the cases, closest == 0 is true, but then I wonder about the lines above and the given comments…

Cheers,

Artur

About this issue

Original URL
State: closed
Created a year ago
Comments: 19 (13 by maintainers)

Most upvoted comments

Yes, sure. Here is the corresponding PR:

https://github.com/cms-sw/cmssw/pull/42108

ArturAkh on Jun 27, 2023

Looking at this code in a past cmssw version vs the present version in master, the reported issue does look like an unintended feature which wasn’t there originally.

For example, if I look at 8_2_X https://github.com/cms-sw/cmssw/blob/CMSSW_8_2_X/RecoEgamma/EgammaPhotonAlgos/src/ConversionHitChecker.cc#L42-L47 , it seems that distance is calculated with it, which is not always the 0-th element.

But, in 8_3_X, https://github.com/cms-sw/cmssw/blob/CMSSW_8_3_X/RecoEgamma/EgammaPhotonAlgos/src/ConversionHitChecker.cc , distance2 is calculated always with the 0-th element. This does not look intended.

Since Nancy mentioned that Josh was the author of this code, let me tag him ( @bendavid )

swagata87 on May 31, 2023

type egamma

swagata87 on May 31, 2023