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

Fix to PFEgammaAlgo #12797

Merged
merged 2 commits into from
Dec 16, 2015
Merged

Fix to PFEgammaAlgo #12797

merged 2 commits into from
Dec 16, 2015

Conversation

matteosan1
Copy link
Contributor

Fix to PFEGammaAlgo to be compliant with PFTrackAlgoTools.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @matteosan1 (Matteo Sani) for CMSSW_8_0_X.

It involves the following packages:

RecoParticleFlow/PFProducer
RecoParticleFlow/PFTracking

@cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @bachtis, @lgray this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@@ -1408,12 +1407,16 @@ initializeProtoCands(std::list<PFEGammaAlgo::ProtoEGObject>& egobjs) {
}
}// loop over tracks in primary vertex
// if associated to good non-GSF matched track remove this cluster
if( Algo < reco::TrackBase::pixelLessStep && nexhits == 0 && fromprimaryvertex ) {
if( PFTrackAlgoTools::isNotPixelLessStep(trackref->algo()) && nexhits == 0 && fromprimaryvertex ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the name is confusing
tobtec etc are "not isNotPixelLessStep" in this case.
It's better to call it by purpose rather than by the actual implementation:
isGoodForEGMPrimary or something like that

@slava77
Copy link
Contributor

slava77 commented Dec 15, 2015

@cmsbuild please test

@makortel was it the only remaining part using iterations?

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10315/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Dec 15, 2015

+1

for #12797 154de34

  • refactored track iteration application; code looks ok
  • jenkins tests pass and comparisons with the baseline show no differences

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@makortel
Copy link
Contributor

@slava77 Yes, this should be the last occurrence inside RecoParticleFlow.

@matteosan1 Thanks!

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Dec 16, 2015
@cmsbuild cmsbuild merged commit f15735a into cms-sw:CMSSW_8_0_X Dec 16, 2015
@matteosan1 matteosan1 deleted the pfegammaalgofix branch April 3, 2016 13:49
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