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

remove bad ES rechits from PF clustering #11873

Merged
merged 6 commits into from
Nov 29, 2015

Conversation

cmsbuild
Copy link
Contributor

remove ES rechits with bad status flag from PF ES clustering
Automatically ported from CMSSW_7_6_X #11858 (original by @cmkuo).

@cmsbuild
Copy link
Contributor Author

A new Pull Request was created by @cmsbuild for CMSSW_8_0_X.

remove bad ES rechits from PF clustering

It involves the following packages:

RecoParticleFlow/PFClusterProducer

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @bachtis, @lgray this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@lgray
Copy link
Contributor

lgray commented Oct 19, 2015

@emanueledimarco

@slava77
Copy link
Contributor

slava77 commented Oct 19, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

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

@cmsbuild
Copy link
Contributor Author

@cmsbuild
Copy link
Contributor Author

@slava77
Copy link
Contributor

slava77 commented Oct 30, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

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

@cmsbuild
Copy link
Contributor Author

Pull request #11873 was updated. @cmsbuild, @cvuosalo, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Nov 23, 2015

@cmkuo
ping
If there are no updates for ~7 more days, we'll request to close this PR.

@cmkuo
Copy link
Contributor

cmkuo commented Nov 24, 2015

@slava77
can you please send me the cmsDriver command and point me to the input file so that I can run reconstruction by myself ? This would speed up my development. Thanks

@slava77
Copy link
Contributor

slava77 commented Nov 24, 2015

Just taking an example from
#11858 (comment)
runTheMatrix.py -l 1330 -i all should reproduce the problem
The plot is suggestive enough what to look at.

@cmkuo
Copy link
Contributor

cmkuo commented Nov 24, 2015

@slava77 thanks. it runs. however, I tried to add a few "cout" in RecoParticleFlow/PFClusterProducer/interface/PFRecHitQTests.h to help me debug.
I do not see anything printed when I run. I do not see anything prevent this to happen in step3_RAW2DIGI_L1Reco_RECO_EI_PAT_VALIDATION_DQM.py
Do you have suggestion where to check why "cout" does not work ?

@slava77
Copy link
Contributor

slava77 commented Nov 24, 2015

plain cout should still come out, if the code is executed.
you can add process.Tracer = cms.Service("Tracer") to see more clearly which module is called at the moment

@cmkuo
Copy link
Contributor

cmkuo commented Nov 25, 2015

@slava77 thanks for the hints. The new fixes were just committed. Now the particleFlowRecHitPS_Cleaned is empty as we want in my local test and PF ES clusters are with bad rechits removal now. Please test.

@cmsbuild
Copy link
Contributor Author

Pull request #11873 was updated. @cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Nov 25, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

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

@cmsbuild
Copy link
Contributor Author

@cmsbuild
Copy link
Contributor Author

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11873/9942/summary.html

@slava77 there are some missing matrix maps:

  • 1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2015

+1

for #11873 4af3e69

  • code changes are in line with the PR description; the latest updates have resolved the issue with the cleaned hit collection side
  • jenkins tests pass and comparisons with baseline show small differences related to egamma objects
  • additionally tested locally in CMSSW_8_0_X_2015-11-23-2300 with more events and more workflows, to also check the technical performance (CPU and disk size).

Reduction in the ES-releated PF clusters and other objects is observed both in MC and in data. The effect is small on high-level objects where a combination with ECAL is made.

wf 25202 (ttbar PU35@25ns)
all_sign618vsorig_ttbarpuwf25202p0c_log10recopfclusters_particleflowclusterps__reco_obj_energy
vs PF photons
all_sign618vsorig_ttbarpuwf25202p0c_log10recopfcandidates_particleflow__reco_obj_pt40
or gedPhotons
all_sign618vsorig_ttbarpuwf25202p0c_recophotons_gedphotons__reco_obj_et

or in Run2015D single photon data (wf 134.911)
all_sign618vsorig_runsingleph2015dwf134p911c_log10recopfclusters_particleflowclusterps__rereco_obj_energy
vs
all_sign618vsorig_runsingleph2015dwf134p911c_log10recopfcandidates_particleflow__rereco_obj_pt40
or
all_sign618vsorig_runsingleph2015dwf134p911c_recophotons_gedphotons__rereco_obj_et

technical changes:

  • large reduction in the number of ES-related objects
    based on wf 134.802 (2015C DoubleEG)
    RECO
 or, B       new, B      delta, B   delta, %   deltaJ, %    branch 
-------------------------------------------------------------
    33565 ->       27550      -6015    -19.7  -0.49     recoPFBlocks_particleFlowBlock__reRECO.
   112945 ->       36467     -76478    -102.4  -6.24     recoPFClusters_particleFlowClusterPS__reRECO.
      726 ->         148       -578    -132.3  -0.05     recoCaloClusters_particleFlowEGamma_ESClusters_reRECO.
...
-------------------------------------------------------------
  1226313 ->     1142555     -83758            -6.8     ALL BRANCHES

The reduction in AOD is also visible (picking the top few)

      881 ->         186       -695    -130.3  -0.32     recoCaloClusters_particleFlowSuperClusterECAL_particleFlowBasicClusterECALPreshower_reRECO.
      726 ->         148       -578    -132.3  -0.26     recoCaloClusters_particleFlowEGamma_ESClusters_reRECO.
...
-------------------------------------------------------------
   218708 ->      217379      -1329            -0.6     ALL BRANCHES

CPU time is reduced as well. In 2015C (134.802 workflow): the total is about 0.2 s/evt or 2% of processing time

   -1.602391      -0.56%        69.30 ms/ev ->         7.65 ms/ev particleFlowClusterPS
   -0.985643      -0.67%       111.57 ms/ev ->        37.90 ms/ev particleFlowRecHitPS
   -0.584455      -0.57%       139.16 ms/ev ->        76.22 ms/ev particleFlowBlock
   -0.491979      -0.11%        29.24 ms/ev ->        17.70 ms/ev particleFlowClusterECAL

In ttbar PU35@25ns (wf 25202) the reduction is larger, about 3% of reco CPU time, with more notable changes in:

        conversionTrackCandidates        594.387 ms/ev -> 552.635 ms/ev
        particleFlowBlock        614.662 ms/ev -> 285.854 ms/ev
        particleFlowEGamma       14.7382 ms/ev -> 12.2066 ms/ev
        particleFlowTmp          54.2423 ms/ev -> 44.9633 ms/ev
        gedPhotons       89.7054 ms/ev -> 52.5367 ms/ev

From the technical performance perspective, this is a very welcome update for 2016 data taking.

@cmsbuild
Copy link
Contributor Author

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

davidlange6 added a commit that referenced this pull request Nov 29, 2015
remove bad ES rechits from PF clustering
@davidlange6 davidlange6 merged commit 63e62d9 into cms-sw:CMSSW_8_0_X Nov 29, 2015
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.

5 participants