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

Puppi MET and MET significance fixes in the MET tool #15565

Merged
merged 12 commits into from Sep 6, 2016

Conversation

mmarionncern
Copy link

@mmarionncern mmarionncern commented Aug 23, 2016

This PR fixes several bugs and inconsistencies in the PAT MET tool :

  • use of proper JECs for the Puppi MET
  • fix the MET significance storage when miniAOD are produced
  • update the MET significance tune
  • modify some MET constructors (RECO and PAT) to keep the met significance matrix when met objects is copied.

Changes expected :

  • Puppi MET and its uncertainties
  • MET significance

The puppi MET results will have sense when #15562 will be integrated.

Packages impacted:

  • DataFormats/METReco
  • DataFormats/PatCandidates
  • PhysicsTools/PatAlgos
  • PhysicsTools/PatUtils
  • RecoMET/METProducers

Local tests and matrix tests seems successfull.

I will be out of office from August 27th to September 17th. If comments comes during that period, please contact @mariadalfonso and @zdemirag .

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmarionncern for CMSSW_8_1_X.

It involves the following packages:

PhysicsTools/PatAlgos
PhysicsTools/PatUtils
RecoMET/METProducers

@cmsbuild, @cvuosalo, @slava77, @montjj, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @imarches, @ahinzmann, @acaudron, @gpetruc, @rappoccio, @jdolen, @nhanvtran, @JyothsnaKomaragiri, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

useRefs = cms.bool(True),
useValueMap = cms.bool(False),
weight = cms.double(1.0),
weightsName = cms.InputTag("puppiNoLep")
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be easier to follow what has changed if parameters were not reshuffled and reindented

@slava77
Copy link
Contributor

slava77 commented Aug 23, 2016

@mmarionncern
please provide a link to a presentation in a POG meeting where these changes are described.

Are you planning a backport to 80X?

@slava77
Copy link
Contributor

slava77 commented Aug 23, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 23, 2016

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

@cmsbuild
Copy link
Contributor

-1

Tested at: 6b2c1be

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15565/14673/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
50202.0 step3

runTheMatrix-results/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/step3_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50.log

@slava77
Copy link
Contributor

slava77 commented Aug 23, 2016

50202 is a 50ns wflow

Exception Message:
No data of type "JME::JetResolutionObject" with label "AK4PFPuppi_pt" in record "JetResolutionRcd"

looks like some condition payloads are missing in the 50ns run2 GT.

@mariadalfonso
Copy link
Contributor

@blinkseb can you help to understand why the AK4PFPuppi_pt is not filled ?
I could access the AK4PFPuppi_phi and the AK4PFPuppi_pt from the Spring16_25nsV6_DATA.db for the plot in the JME-16-004

@mariadalfonso
Copy link
Contributor

@slava77 these fixes went in the plot for the JME-16-004.
We must indeed backport into the 80X for the re-reco (as soon we are sure we can integrate this one)

@cmsbuild
Copy link
Contributor

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

1 similar comment
@cmsbuild
Copy link
Contributor

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

@mmarionncern
Copy link
Author

Two extra commits touching the data formats have been added to keep the MET significance matrix when the MET is copied (both RECO and PAT level)
Description is updated

@mmusich
Copy link
Contributor

mmusich commented Aug 25, 2016

@mariadalfonso missing conditions are in the 50ns Run2 MC Global Tag. The Spring16_25nsV6 set of payloads has not been propagated to 50ns (which is not 25ns ...). I don't think it makes too much sense change the 50ns GT using the latest 25ns JECs. Maybe just adding the labels for the resolutions: what do you think?
Side note: testing the PR with runTheMatrix.py -l limited before submitting is always a good practice.

@slava77
Copy link
Contributor

slava77 commented Aug 26, 2016

@mmarionncern @mariadalfonso @mmusich
has there been progress in getting the AK4PFPuppi_pt payolads in all the right places?

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2016

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

@mariadalfonso
Copy link
Contributor

@slava77 rebased interactively picking the right veelken commit,

@slava77
Copy link
Contributor

slava77 commented Sep 5, 2016

@cmsbuild please test

@mariadalfonso it looks like we are almost there.
I will check this at run time now.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2016

-1

Tested at: 7f89d67

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15565/14950/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
4.22 step1

DAS Error

@slava77
Copy link
Contributor

slava77 commented Sep 5, 2016

@cmsbuild please test
retry, hoping to get rid of the DAS error

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2016

@slava77
Copy link
Contributor

slava77 commented Sep 6, 2016

+1

for #15565 b742cc1

  • code changes are in line with the description and the follow up review. Main changes are to appropriately select jet correctors in Puppi corrected MET and also to set MET significance (with updated parameters to compute the significance).
  • jenkins tests pass and comparisons with baseline show differences in miniAOD METs
  • local tests confirm the changes observed in jenkins tests: there are minor changes in slimmedMETsPuppi p3 (due to updates to T1 correctors), and also the significance variable in the MET product is now filled (instead of -1) in slimmedMETs and slimmedMETsPuppi
    • net CPU use is roughly unchanged
    • updates in miniAOD show ~0.1% increase in file size (based on 70 events in ttbar PU35, which is probably an overestimate, given the small number of events tested)

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

8 participants