L1 prefiring weight producer #25380

lathomas · 2018-11-29T22:37:24Z

Dear CMSSW managers/PR reviewers,

This PR contains an EDProducer computing weights associated to the L1 ECAL prefiring issue that affected 2016 and 2017 data.
Since the effect is not present in MC, the proposed solution is to reweight simulations by those weights.
I add some links to documentation at the end of this post for more information.

The code is a producer returning three double (central, up/down weights). The code loops over all photons and jets found in the event and multiplies their prefiring probabilities.

I expect the PR to need a bit of iteration before being approved.
In particular, I would greatly appreciate your suggestions regarding the following points:

Can you please provide advise regarding the code location? I created a new package but it can/should maybe enter an already existing one?
The code relies on prefiring maps stored as TH2F in a root file. I'm not sure this is allowed... If not, can you suggest a proper format? (sorry for my ignorance).

Please let me know if anything else is unclear,

Thanks a lot,

Laurent

Documentation
-Talk at CMS week about prefiring:
https://indico.cern.ch/event/704625/contributions/3164334/attachments/1726987/2790093/prefiringupdateCMSweek.pdf
-Prefiring discussion at the XPOG forum:
https://indico.cern.ch/event/764279/
-Twiki describing how to run the producer:
https://twiki.cern.ch/twiki/bin/viewauth/CMS/L1ECALPrefiringWeightRecipe
-L1 prefiring maps per object:
https://lathomas.web.cern.ch/lathomas/TSGStuff/L1Prefiring/PrefiringMaps_2016and2017/
-Validation plots:
https://twiki.cern.ch/twiki/pub/CMS/L1ECALPrefiringWeightRecipe/prefiring_producervalidation.pdf

cmsbuild · 2018-11-29T22:37:47Z

The code-checks are being triggered in jenkins.

lathomas · 2018-11-29T22:39:35Z

@peruzzim @fgolf

cmsbuild · 2018-11-29T22:41:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25380/7414

cmsbuild · 2018-11-29T22:43:37Z

A new Pull Request was created by @lathomas for master.

It involves the following packages:

L1Prefiring/EventWeightProducer

The following packages do not have a category, yet:

L1Prefiring/EventWeightProducer
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild can you please review it and eventually sign? Thanks.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

peruzzim · 2018-11-30T09:09:38Z

If the review and backport to 10_2_X proceed in an expedite way, this could get into the next NanoAOD production, which would be very valuable.

davidlange6 · 2018-11-30T09:22:56Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+   for(int fluct = 0; fluct<3;fluct++) {
+     //Start by applying the prefiring maps to photons in the affected regions. 
+     std::vector < pat::Photon > affectedphotons; 
+     for( std::vector<pat::Photon>::const_iterator photon = (*thePhotons).begin(); photon != (*thePhotons).end(); photon++ ) {


this can be more cleanly/efficiently written as
for ( const auto & photon : *thePhotons ) {
double pt_gam = photon.pt();

etc

davidlange6 · 2018-11-30T09:25:06Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+       if( pt_gam < 20.) continue;
+       if( fabs(eta_gam) <2.) continue;
+       if( fabs(eta_gam) >3.) continue;
+       affectedphotons.push_back((*photon));


consider storing a const & or just a vector of bools instead of copying the photon object here

davidlange6 · 2018-11-30T09:26:01Z

how big is the TH2F?

davidlange6 · 2018-11-30T09:26:51Z

probably PhysicsTools/PatUtils is a good home for this code

peruzzim

In order to speed up the review I give a few comments from a quick look at the code

peruzzim · 2018-11-30T09:18:25Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+       if( pt_gam < 20.) continue;
+       if( fabs(eta_gam) <2.) continue;
+       if( fabs(eta_gam) >3.) continue;
+       affectedphotons.push_back((*photon));


I think we want to avoid copies by value here

peruzzim · 2018-11-30T09:19:51Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+  edm::EDGetTokenT<std::vector< pat::Jet> > jets_token_;
+
+
+  TH2F * h_prefmap_photon; 


where does this get deleted? should we use smart pointers?

peruzzim · 2018-11-30T09:20:30Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+
+  TFile *  file_prefiringmaps_;
+  std::string fname =  iConfig.getParameter<std::string>( "L1Maps" );
+  file_prefiringmaps_ = new TFile(  fname.c_str(),"read" );


I think we want to use FileInPath to eventually read from different locations here

peruzzim · 2018-11-30T09:21:03Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+  std::string fname =  iConfig.getParameter<std::string>( "L1Maps" );
+  file_prefiringmaps_ = new TFile(  fname.c_str(),"read" );
+  TString mapphotonfullname= "L1prefiring_photonptvseta_"+ dataera_; 
+  h_prefmap_photon =(TH2F*) file_prefiringmaps_->Get(mapphotonfullname);


needs protection/check if the file does not contain the histogram with that name

peruzzim · 2018-11-30T09:21:45Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+  //  virtual void beginJob();
+  //virtual void endJob(void);
+  void endStream() override;
+  virtual double GetPrefiringRate( double eta, double pt, TH2F * h_prefmap,/*std::string object, std::string dataera, */ int fluctuation);


here it's probably cleaner to pass the enum type and not the int

peruzzim · 2018-11-30T09:24:01Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+
+       }
+       //If overlapping photons have a non prefiring rate smaller than the jet, don't consider the jet in the event weight
+       else if(nonprefiringprobfromoverlappingphotons < nonprefiringprobfromoverlappingjet ) NonPrefiringProba[fluct]*=1.;


what is this line actually doing? (multiply by 1)

peruzzim · 2018-11-30T09:24:35Z

L1Prefiring/EventWeightProducer/plugins/L1ECALPrefiringWeightProducer.cc

+
+double L1ECALPrefiringWeightProducer::GetPrefiringRate( double eta, double pt, TH2F * h_prefmap /*std::string object, std::string dataera*/ , int fluctuation){
+
+  if(h_prefmap==nullptr) return 0.;


this is dangerous as it fails silently in case of a configuration mistake

cmsbuild · 2018-11-30T13:22:22Z

The code-checks are being triggered in jenkins.

lathomas · 2018-11-30T13:27:59Z

@peruzzim @davidlange6
I tried to address all your comments and moved the code to PhysicsTools/PatUtils.
The root file size is 15kB. Judging by the size of other files in PhysicsTools/PatUtils/data I assume this is fine (?)

Thanks !

cmsbuild · 2018-11-30T13:34:33Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25380/7420

cmsbuild · 2019-01-10T14:46:50Z

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

perrotta · 2019-01-10T14:50:24Z

please test

cmsbuild · 2019-01-10T14:50:50Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32516/console Started: 2019/01/10 15:52

cmsbuild · 2019-01-10T16:19:59Z

+1
Tested at: 793d75e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25380/32516/summary.html

cmsbuild · 2019-01-10T16:20:05Z

Comparison job queued.

cmsbuild · 2019-01-10T17:51:31Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 33
DQMHistoTests: Total histograms compared: 3153717
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3153512
DQMHistoTests: Total skipped: 204
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
Checked 137 log files, 14 edm output root files, 33 DQM output files

perrotta · 2019-01-11T10:30:13Z

@lathomas : do you have an analyzer (which runs on nanoAOD, possibly) that consumes these weights? I.e. something obtained with https://twiki.cern.ch/twiki/bin/viewauth/CMS/L1ECALPrefiringWeightRecipe but avoiding me the need to recreate everything from scratch following that recipe.
Once I can verify that this PR can run correctly even after the last updates I will sign it.

perrotta · 2019-01-11T16:48:26Z

+1

Already signed (see L1 prefiring weight producer #25380 (comment)), then cfg fixed, rebased, and a few further adjustments were applied following the code review
This is an analysis tool not used by default in any official workflow, but meant for now to private analyses and eventually possibly also nanoAOD production
By following the recipe provided, it runs (I managed to do it on nanoAOD)
The code reflects what declared, and subsequent review comments
Jenkins tests pass and show no differences
Needed external got already merged in the master

lathomas · 2019-01-11T18:21:24Z

@perrotta : thanks !

fabiocos · 2019-01-12T14:55:30Z

+1

the configuration of the module has been properly adjusted, thanks

fabiocos · 2019-01-12T14:55:40Z

merge

lathomas · 2019-01-14T09:36:43Z

@fabiocos @perrotta @peruzzim
Thanks for the merging and for your help.
Can I proceed with the backport? What branches should I consider? I guess at least 94X is needed but are there others?

perrotta · 2019-01-14T09:44:23Z

Analyses will be performed in 10_2_X (preferably), and also in 94X: please consider backporting in both cycles. lathomas <notifications@github.com> ha scritto:

…

@fabiocos @perrotta @peruzzim Thanks for the merging and for your help. Can I proceed with the backport? What branches should I consider? I guess at least 94X is needed but are there others? -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #25380 (comment)

mrodozov · 2019-01-14T13:31:21Z

@perrotta @lathomas
cms-sw/cmsdist#4635
cms-sw/cmsdist#4636
are for 10_2 and 9_4

perrotta · 2019-01-14T13:43:04Z

Thank you Mircho! Mircho Rodozov <notifications@github.com> ha scritto:

…

@perrotta @lathomas cms-sw/cmsdist#4635 cms-sw/cmsdist#4636 are for 10_2 and 9_4 -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #25380 (comment)

cmsbuild added this to the CMSSW_10_4_X milestone Nov 29, 2018

cmsbuild added code-checks-pending comparison-pending new-package-pending orp-pending pending-signatures tests-pending labels Nov 29, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 29, 2018

davidlange6 reviewed Nov 30, 2018

View reviewed changes

peruzzim reviewed Nov 30, 2018

View reviewed changes

peruzzim mentioned this pull request Nov 30, 2018

Prototype implementation of L1 prefiring event weight in NanoAOD cms-nanoAOD/cmssw#266

Closed

cmsbuild added analysis-pending code-checks-pending reconstruction-pending and removed code-checks-approved new-package-pending labels Nov 30, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 30, 2018

cmsbuild added the code-checks-approved label Jan 10, 2019

cmsbuild added tests-started and removed tests-pending labels Jan 10, 2019

cmsbuild added tests-approved and removed tests-started labels Jan 10, 2019

cmsbuild added comparison-available and removed comparison-pending labels Jan 10, 2019

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jan 11, 2019

cmsbuild added orp-approved and removed orp-pending labels Jan 12, 2019

cmsbuild merged commit 842f8a0 into cms-sw:master Jan 12, 2019

This was referenced Jan 14, 2019

L1 Prefiring weight extractor (backport to 10_2_X) #25645

Merged

L1 Prefiring weight extractor (backport to 9_4_X) #25646

Merged

bortigno mentioned this pull request Mar 5, 2019

Add L1 pre-firing weights UFLX2MuMu/Ntupliser#50

Closed

		edm::EDGetTokenT<std::vector< pat::Jet> > jets_token_;


		TH2F * h_prefmap_photon;


		double L1ECALPrefiringWeightProducer::GetPrefiringRate( double eta, double pt, TH2F * h_prefmap /std::string object, std::string dataera/ , int fluctuation){

		if(h_prefmap==nullptr) return 0.;

L1 prefiring weight producer #25380

L1 prefiring weight producer #25380

Conversation

lathomas commented Nov 29, 2018

cmsbuild commented Nov 29, 2018

lathomas commented Nov 29, 2018

cmsbuild commented Nov 29, 2018

cmsbuild commented Nov 29, 2018

peruzzim commented Nov 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidlange6 commented Nov 30, 2018

davidlange6 commented Nov 30, 2018

peruzzim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Nov 30, 2018

lathomas commented Nov 30, 2018

cmsbuild commented Nov 30, 2018

cmsbuild commented Jan 10, 2019

perrotta commented Jan 10, 2019

cmsbuild commented Jan 10, 2019 • edited

cmsbuild commented Jan 10, 2019

cmsbuild commented Jan 10, 2019

cmsbuild commented Jan 10, 2019

perrotta commented Jan 11, 2019

perrotta commented Jan 11, 2019

lathomas commented Jan 11, 2019

fabiocos commented Jan 12, 2019

fabiocos commented Jan 12, 2019

lathomas commented Jan 14, 2019

perrotta commented Jan 14, 2019 via email

mrodozov commented Jan 14, 2019

perrotta commented Jan 14, 2019 via email

cmsbuild commented Jan 10, 2019 •

edited