update PUPPI based on DP note #10597

nhanvtran · 2015-08-05T21:19:30Z

This is the companion PR to #10551

nhanvtran · 2015-08-05T21:20:25Z

This supercedes #10404

cmsbuild · 2015-08-05T21:21:12Z

A new Pull Request was created by @nhanvtran for CMSSW_7_5_X.

update PUPPI based on DP note

It involves the following packages:

CommonTools/PileupAlgos
DataFormats/PatCandidates
PhysicsTools/PatAlgos

@cmsbuild, @cvuosalo, @vadler, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @imarches, @ahinzmann, @acaudron, @mmarionncern, @jdolen, @schoef, @ferencek, @gpetruc, @mariadalfonso, @pvmulder, @TaiSakuma 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.

slava77 · 2015-08-05T22:00:59Z

@cmsbuild please test

cmsbuild · 2015-08-05T22:01:10Z

The tests are being triggered in jenkins.

cmsbuild · 2015-08-06T00:45:39Z

+1
Tested at: 4466938
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-10597/7002/summary.html

cmsbuild · 2015-08-06T01:49:14Z

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

There are some workflows for which there are errors in the baseline:
50202.0 step 2
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

slava77 · 2015-08-13T15:18:36Z

ping

nhanvtran · 2015-08-17T17:43:07Z

re-indenting updated

cmsbuild · 2015-08-17T17:46:32Z

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

slava77 · 2015-08-17T18:29:19Z

CommonTools/PileupAlgos/plugins/PuppiProducer.cc

-
-      //std::cout << "pVtxId = " << pVtxId << ", and charge = " << itPF->charge() << ", and closestVtx = " << closestVtx << ", and id = " << pReco.id << std::endl;
-      //std::cout << "charge = " << itPF->charge() << ", pDZ = " << pDZ << ", pVtxId = " << pVtxId << ", closestVtx = " << closestVtx << ", fromPV() = " << lPack->fromPV() << ", pReco.id = " << pReco.id << std::endl;
+    //   //std::cout << "pVtxId = " << pVtxId << ", and charge = " << itPF->charge() << ", and closestVtx = " << closestVtx << ", and id = " << pReco.id << std::endl;


are these debugs still really needed?

slava77 · 2015-08-19T16:01:05Z

PhysicsTools/PatAlgos/plugins/PATPackedCandidateProducer.cc

+             for ( size_t ipcnl = 0; ipcnl < puppiCandsNoLepPtrs.size(); ipcnl++){
+              if (puppiCandsNoLepPtrs[ipcnl] == pkrefPtr){
+                foundNoLep = true;
+                  puppiWeightNoLepVal = puppiCandsNoLepPtrs[ipcnl]->pt()/cand.pt(); // a hack for now, should use the value map


the previous code was picking pt from the puppiCandsNoLep, not from the source ptr.
If it matters functionally, you should probably have

// define outside the loop: auto const& puppiWeightNoLepV = puppiWeightNoLep.product(); puppiWeightNoLepVal = puppiWeightNoLepV[ipcnl].pt()/cand.pt();

nhanvtran · 2015-08-19T18:53:55Z

Ok @slava77 updated

slava77 · 2015-08-19T18:56:02Z

@cmsbuild please test

cmsbuild · 2015-08-19T18:56:38Z

The tests are being triggered in jenkins.

cmsbuild · 2015-08-19T18:56:39Z

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

cmsbuild · 2015-08-19T19:11:48Z

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

slava77 · 2015-08-19T19:22:51Z

@cmsbuild please test

cmsbuild · 2015-08-19T19:26:53Z

The tests are being triggered in jenkins.

cmsbuild · 2015-08-19T21:16:20Z

+1
Tested at: 35e96ca
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-10597/7480/summary.html

cmsbuild · 2015-08-19T21:53:14Z

+1
Tested at: 06dcff6
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-10597/7483/summary.html

cmsbuild · 2015-08-19T22:13:12Z

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

cmsbuild · 2015-08-19T22:49:18Z

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

slava77 · 2015-08-19T22:56:09Z

+1

for #10597 06dcff6

code changes are OK
jenkins tests pass and comparisons with baseline show no differences
additionally tested in CMSSW_7_5_X_2015-08-18-2300 /test area sign575/ with PAT step only running on 70 events input from ttbar PU=35@25ns
- CPU increase is now more acceptable:
  - PuppiProducer went up from by 35% (two producers are running now), which is achieved by removal of PuppiContainer::getRawAlphas calls (previously 40% of the PuppiProducer)
  - pat::PATPackedCandidateProducer increase in CPU is now only 70% (from x20 in the last iteration) https://slava77sk.web.cern.ch/slava77sk/reco/cgi-bin/igprof-navigator/CMSSW_7_5_X_2015-08-18-2300-sign575.06dcff6.25202.0.step5.pp/595 vs https://slava77sk.web.cern.ch/slava77sk/reco/cgi-bin/igprof-navigator/CMSSW_7_5_X_2015-08-18-2300-orig.25202.0.step5.pp/807
    - here a large fraction of the increase seems to be from reading the additional collections and some manipulations.
    - Also, the compiler decided to inline the PackedCandidate constructor, which makes the comparison difficult.
- compared to baseline the number of puppi jets is smaller (as expected, see Puppi 75x miniaodpass2 #10404 ). packed candidates get smaller, but this may be incidental

-------------------------------------------------------------
 or, B       new, B      delta, B   delta, %   deltaJ, %    branch 
-------------------------------------------------------------
    19054 ->       18904       -150     -0.8  -0.21     patPackedCandidates_packedPFCandidates__PAT.
     2723 ->        2291       -432    -17.2  -0.60     patJets_slimmedJetsPuppi__PAT.

update PUPPI based on DP note

update PUPPI based on DP note

4466938

cmsbuild added this to the Next CMSSW_7_5_X milestone Aug 5, 2015

cmsbuild added analysis-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 5, 2015

cmsbuild added tests-started and removed tests-pending labels Aug 5, 2015

cmsbuild added tests-approved and removed tests-started labels Aug 6, 2015

cmsbuild added comparison-available and removed comparison-pending labels Aug 6, 2015

reindenting

7aa2807

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels Aug 17, 2015

slava77 reviewed Aug 17, 2015
View reviewed changes

slava77 reviewed Aug 19, 2015
View reviewed changes

one more small optimization

35e96ca

cmsbuild added tests-started and removed tests-pending labels Aug 19, 2015

diagnostics turned off to speed it up

06dcff6

cmsbuild added tests-pending and removed tests-started labels Aug 19, 2015

cmsbuild added tests-started and removed tests-pending labels Aug 19, 2015

cmsbuild added tests-approved and removed tests-started labels Aug 19, 2015

cmsbuild added comparison-available and removed comparison-pending labels Aug 19, 2015

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Aug 19, 2015

davidlange6 added a commit that referenced this pull request Aug 20, 2015

Merge pull request #10597 from nhanvtran/puppi-75x-miniaodpass2-v2

ce2b921

update PUPPI based on DP note

davidlange6 merged commit ce2b921 into cms-sw:CMSSW_7_5_X Aug 20, 2015

nhanvtran mentioned this pull request Aug 24, 2015

updating PUPPI, 76x #10933

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update PUPPI based on DP note #10597

update PUPPI based on DP note #10597

nhanvtran commented Aug 5, 2015

nhanvtran commented Aug 5, 2015

cmsbuild commented Aug 5, 2015

slava77 commented Aug 5, 2015

cmsbuild commented Aug 5, 2015

cmsbuild commented Aug 6, 2015

cmsbuild commented Aug 6, 2015

slava77 commented Aug 13, 2015

nhanvtran commented Aug 17, 2015

cmsbuild commented Aug 17, 2015

slava77 Aug 17, 2015

nhanvtran Aug 18, 2015

slava77 Aug 19, 2015

nhanvtran commented Aug 19, 2015

slava77 commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

slava77 commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

slava77 commented Aug 19, 2015

update PUPPI based on DP note #10597

update PUPPI based on DP note #10597

Conversation

nhanvtran commented Aug 5, 2015

nhanvtran commented Aug 5, 2015

cmsbuild commented Aug 5, 2015

slava77 commented Aug 5, 2015

cmsbuild commented Aug 5, 2015

cmsbuild commented Aug 6, 2015

cmsbuild commented Aug 6, 2015

slava77 commented Aug 13, 2015

nhanvtran commented Aug 17, 2015

cmsbuild commented Aug 17, 2015

slava77 Aug 17, 2015

Choose a reason for hiding this comment

nhanvtran Aug 18, 2015

Choose a reason for hiding this comment

slava77 Aug 19, 2015

Choose a reason for hiding this comment

nhanvtran commented Aug 19, 2015

slava77 commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

slava77 commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

cmsbuild commented Aug 19, 2015

slava77 commented Aug 19, 2015