cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.09k stars 4.33k forks source link

PFEGammaAlgo: Need to implement proper merging of two gsf candidates! #23216

Open slava77 opened 6 years ago

slava77 commented 6 years ago

There is a LogWarning, which is serving as a reminder of a to-do item in https://github.com/cms-sw/cmssw/blob/CMSSW_10_2_0_pre3/RecoParticleFlow/PFProducer/src/PFEGammaAlgo.cc

This is happening in a corner case and was on the to-do list since 2013

     if( thefront.electronSeed.isNull() && 
         roToMerge->electronSeed.isNonnull() ) {
//...
     } else if ( thefront.electronSeed.isNonnull() && 
             roToMerge->electronSeed.isNonnull()) {
       LOGWARN("PFEGammaAlgo::mergeROsByAnyLink")
         << "Need to implement proper merging of two gsf candidates!"
         << std::endl;
}

@Sam-Harper @jainshilpi @varuns23

cmsbuild commented 6 years ago

A new Issue was created by @slava77 Slava Krutelyov.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

slava77 commented 6 years ago

assign reconstruction

cmsbuild commented 6 years ago

New categories assigned: reconstruction

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

Sam-Harper commented 6 years ago

@lgray, @beaudett What is actually meant by this comment. I assume its from you guys? Is it something that should be fixed?

beaudett commented 6 years ago

Hi Sam, I don't think that this comment was from me and I actually don't know what it means. Sorry. Florian

Sam-Harper commented 6 years ago

No problem, thanks for the quick response. I suspect Lindsey then :D

perrotta commented 6 years ago

The "blame" option in github points to Lindsey indeed (in a commit from some 5 years ago...): https://github.com/cms-sw/cmssw/commit/d4958ba2923cb58dffc7a40f8b059a638c810462

lgray commented 6 years ago

Hi Guys,

So - right now if two candidates with GSF tracks are found to overlap (point to the same set of block elements) the one being merged into is simply erased. This frees up the block elements to be absorbed into other remaining e/gamma objects (iirc). This is pretty rare and usually only involves duplicate tracks, but there are occasional physical reasons for this happening.

Some arbitration logic would be needed or even some simple selection as to which GSF track is better and then winner can take all. This is both delicate and reasonably rare (at least in the past detector and tracking configuration), which is why we never covered the case.

slava77 commented 5 years ago

@Sam-Harper @bendavid @hatakeyamak is this topic on the list of features for the refactored/refurbished PFEGammaAlgo?

hatakeyamak commented 5 years ago

I wasn't aware of this, but seems like something to keep in mind. @Sam-Harper @bendavid what do you think?

slava77 commented 3 years ago

@hatakeyamak @bendavid @afiqaize @SohamBhattacharya I'm curious if there were some developments here in the last two years

afiqaize commented 3 years ago

this is the first time I've become aware of this issue, so no, I'm not aware of any developments.

is this becoming more of a problem now?