Updating Puppiproducer to include fillDescriptions v2 #28020

alefisico · 2019-09-18T13:35:49Z

PR description:

To test PUPPI at HLT for Run3/PhaseII, PuppiProducer has to be updated with the fillDescription function to be recognized in condfb. This PR introduces this function in that producer. This change should be completely transparent to the PUPPI algorithm itself, or any other part of CMSSW.

PR validation:

Run runTheMatrix.py -l limited -i all --ibeos, and all the test passed. Here is the output of it:
https://cernbox.cern.ch/index.php/s/tzHW6iQMrXSmKY4

This is the master branch of this PR #28016

cmsbuild · 2019-09-18T13:36:21Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-09-18T13:43:54Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28020/11930

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CommonTools/PileupAlgos/plugins/PuppiProducer.cc modified in PR(s): Simplify PUPPI Photon producer and change PUPPI isolation algo #27929

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28020/11930/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28020/11930/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2019-09-18T13:47:30Z

The code-checks are being triggered in jenkins.

perrotta · 2019-09-18T13:51:12Z

This is going to interfere with @ahinzmann 's #27929: one has to decide who needs to go first

alefisico · 2019-09-18T13:52:59Z

for sure #27929 is more important than this PR. This can go after that

cmsbuild · 2019-09-18T13:53:26Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28020/11931

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CommonTools/PileupAlgos/plugins/PuppiProducer.cc modified in PR(s): Simplify PUPPI Photon producer and change PUPPI isolation algo #27929

cmsbuild · 2019-09-18T13:53:52Z

A new Pull Request was created by @alefisico (Alejandro Gomez Espinosa) for master.

It involves the following packages:

CommonTools/PileupAlgos

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @ahinzmann, @riga, @jdolen, @gkasieczka, @clelange this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

perrotta · 2019-09-18T13:59:01Z

CommonTools/PileupAlgos/plugins/PuppiProducer.cc

+  desc.add<bool>("useExp", false);
+  desc.add<double>("MinPuppiWeight", .01);
+
+  edm::ParameterSetDescription puppialgos;


All PuppiAlgo related PSet's should get defined inside the PuppiAlgo class, not here in the caller

ahinzmann · 2019-09-18T13:59:54Z

#27929 is just a nice to have and needs further development as it degrades CPU performance, so #28020 should go first.

perrotta · 2019-09-19T16:16:50Z

CommonTools/PileupAlgos/plugins/PuppiProducer.cc

+  puppialgos.add<double>("rmsScaleFactor", 1.0);
+
+  edm::ParameterSetDescription algos;
+  std::vector<double> etaMin;


Defining uninitialized vectors is useless. You could just define a single dummy vector, e.g.

std::vector<double> dummy_vdouble;

and use it to add everywhere.

Or (probaly better?) you can fill with some reasonable value, e.g. the ones from CommonTools/PileupAlgos/python/Puppi_cff.py:

algos.add<std::vector<double>>("etaMin", {0.}); algos.add<std::vector<double>>("etaMax", {2.5}); algos.add<std::vector<double>>("ptMin", {0.}); algos.add<std::vector<double>>("MinNeutralPt", {0.2});

etc.

By the way: this default algo configuration corresponds to what? I couldn't find it in the actual config in cmssw

cmsbuild · 2019-10-03T09:35:01Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28020/12120

This PR adds an extra 76KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CommonTools/PileupAlgos/interface/PuppiAlgo.h modified in PR(s): Make puppi algo better readable #28104
- File CommonTools/PileupAlgos/plugins/PuppiProducer.cc modified in PR(s): Simplify PUPPI Photon producer and change PUPPI isolation algo #27929
- File CommonTools/PileupAlgos/plugins/PuppiProducer.h modified in PR(s): Simplify PUPPI Photon producer and change PUPPI isolation algo #27929
- File CommonTools/PileupAlgos/src/PuppiAlgo.cc modified in PR(s): Make puppi algo better readable #28104

cmsbuild · 2019-10-03T09:35:34Z

Pull request #28020 was updated. @perrotta, @cmsbuild, @santocch, @slava77 can you please check and sign again.

perrotta · 2019-10-03T10:02:53Z

please test

cmsbuild · 2019-10-03T10:03:22Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2772/console Started: 2019/10/03 12:05

cmsbuild · 2019-10-03T11:17:36Z

+1
Tested at: 358e4d2
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-64e379/2772/summary.html

cmsbuild · 2019-10-03T11:17:40Z

Comparison job queued.

cmsbuild · 2019-10-03T13:11:01Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2961049
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2960706
DQMHistoTests: Total skipped: 341
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

perrotta · 2019-10-03T22:02:52Z

+1

Configuration description added, and the produced config was verified to contain all needed parameters
Jenkins tests pass and show no differences, as it should

fabiocos · 2019-10-05T21:48:41Z

+1

fabiocos · 2019-10-05T21:48:48Z

merge

santocch · 2019-10-17T18:47:09Z

+1

cmsbuild · 2019-10-17T18:47:39Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged.

including fillDescriptions in PuppiProducer

1c04c6d

alefisico mentioned this pull request Sep 18, 2019

Updating Puppiproducer to include fillDescriptions #28016

Closed

cmsbuild added this to the CMSSW_11_0_X milestone Sep 18, 2019

cmsbuild added analysis-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Sep 18, 2019

cmsbuild added code-checks-rejected and removed code-checks-pending labels Sep 18, 2019

fixing code-format

bf37221

cmsbuild added code-checks-pending and removed code-checks-rejected labels Sep 18, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 18, 2019

perrotta reviewed Sep 18, 2019

View reviewed changes

cmsbuild mentioned this pull request Sep 19, 2019

Simplify PUPPI Photon producer and change PUPPI isolation algo #27929

Merged

adding fillDescriptions

cb186be

perrotta reviewed Sep 19, 2019

View reviewed changes

cmsbuild mentioned this pull request Sep 20, 2019

Fix a discrepancy in Puppi weights when running on MiniAOD w/ useExistingWeights=False #28033

Merged

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 3, 2019

cmsbuild added tests-started and removed tests-pending labels Oct 3, 2019

cmsbuild added tests-approved and removed tests-started labels Oct 3, 2019

cmsbuild added comparison-available and removed comparison-pending labels Oct 3, 2019

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Oct 3, 2019

cmsbuild added orp-approved and removed orp-pending labels Oct 5, 2019

cmsbuild merged commit 42f7ffc into cms-sw:master Oct 5, 2019

alefisico mentioned this pull request Oct 9, 2019

Updating Puppiproducer to include fillDescriptions (backport 106X) #28137

Merged

cmsbuild removed the analysis-pending label Oct 17, 2019

cmsbuild added analysis-approved fully-signed and removed pending-signatures labels Oct 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating Puppiproducer to include fillDescriptions v2 #28020

Updating Puppiproducer to include fillDescriptions v2 #28020

alefisico commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

perrotta commented Sep 18, 2019

alefisico commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

perrotta Sep 18, 2019

ahinzmann commented Sep 18, 2019

perrotta Sep 19, 2019

perrotta Sep 19, 2019

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

perrotta commented Oct 3, 2019

cmsbuild commented Oct 3, 2019 •

edited

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

perrotta commented Oct 3, 2019

fabiocos commented Oct 5, 2019

fabiocos commented Oct 5, 2019

santocch commented Oct 17, 2019

cmsbuild commented Oct 17, 2019

Updating Puppiproducer to include fillDescriptions v2 #28020

Updating Puppiproducer to include fillDescriptions v2 #28020

Conversation

alefisico commented Sep 18, 2019

PR description:

PR validation:

cmsbuild commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

perrotta commented Sep 18, 2019

alefisico commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

perrotta Sep 18, 2019

Choose a reason for hiding this comment

ahinzmann commented Sep 18, 2019

perrotta Sep 19, 2019

Choose a reason for hiding this comment

perrotta Sep 19, 2019

Choose a reason for hiding this comment

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

perrotta commented Oct 3, 2019

cmsbuild commented Oct 3, 2019 • edited

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

perrotta commented Oct 3, 2019

fabiocos commented Oct 5, 2019

fabiocos commented Oct 5, 2019

santocch commented Oct 17, 2019

cmsbuild commented Oct 17, 2019

cmsbuild commented Oct 3, 2019 •

edited