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

Fix Puppi MET and smearing MET correction, add central test for the MET tool #13954

Merged
merged 8 commits into from Apr 18, 2016

Conversation

mmarionncern
Copy link

This PR fixes only configuration : the computation of the puppiMET in the miniAOD production, now using the proper jet energy corrections for the puppiMET and a small inconsistency in the MET smearing correction have been also added.
A central test for the MET tool has been added, in order to avoid conflicts such as the one seen with the update of the PAT names few weeks ago.
Few internal variable names were also modified.

The miniAOD to miniAOD MET reprocessing example test file has been updated and a test file for the MET production from AOD has been added.

Packages impacted by this PR :

  • PhysicsTools/PatAlgos
  • PhysicsTools/PatUtils

Changes :

  • puppiMET will change due to the modification of the jet energy corrections used
  • the smeared T1 PFMET is changing, no other change expected for the slimmedMET content
  • no modification of the CPU/memory performances for the miniAOD production sequence
  • fix MET unclustered uncertainty bug
  • fix jet smearing bug

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

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

It involves the following packages:

PhysicsTools/PatAlgos
PhysicsTools/PatUtils

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

cms-bot commands are list here #13028

@slava77
Copy link
Contributor

slava77 commented Apr 6, 2016

@cmsbuild please test
... this is expected to go to the first re-miniAOD version, unlikely sooner.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

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

@mmarionncern
Copy link
Author

@slava77 those fixes are quite minor ones, no problem of not having them super-soon

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

-1
Tested at: d6edab2
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
----- Begin Fatal Exception 06-Apr-2016 15:22:37 CEST-----------------------
An exception of category 'InvalidInput' occurred while
   [0] Processing run: 1 lumi: 1 event: 1
   [1] Running path 'validation_step1'
   [2] Calling event method for module METTester/'pfPuppiMetAnalyzerMiniAOD'
   [3] Calling produce method for unscheduled module PATMETSlimmer/'slimmedMETsPuppi'
   [4] Calling produce method for unscheduled module CorrectedPATMETProducer/'patPFMetT1JetEnUpPuppi'
   [5] Calling produce method for unscheduled module ShiftedParticleMETcorrInputProducer/'shiftedPatMETCorrJetEnUpPuppi'
   [6] Calling produce method for unscheduled module ShiftedPATJetProducer/'shiftedPatJetEnUpPuppi'
Exception Message:
 cannot find key 9 in the JEC payload, this usually means you have to change the global tag
----- End Fatal Exception -------------------------------------------------

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

@gpetruc
Copy link
Contributor

gpetruc commented Apr 6, 2016

could we split off the part that affects miniAOD production from the one that does not?
and understand whether the first part is something small enough that could go in v2 or not?

I don't like the idea of producing miniAOD v2 mainly to put in some new JECs, but do that with a release that applies them wrongly.

@arizzi

@mmarionncern
Copy link
Author

The matrix test fails due to the use of the GT pointed by " auto:run2_mc_50ns" which does not contains the Puppi JEC. I don't know what is the proper action to take : should they be added to that GT or should the _50ns be removed from the GT pointer (the matrix test works with auto:run2_mc)?

@mmarionncern
Copy link
Author

@gpetruc @arizzi

The changes affecting the miniAOD production is half of the PR in term of changes :
-> puppi JEC corrections for puppiMET
-> MET smearing correction (minor correction and not needed for a new production, but still affecting it)

The other ones are just and update of the test files and a new unit test.

The separation between both is feasible, but it means I will have to split the single commit we have right now.

@mmarionncern
Copy link
Author

@slava77 @mmusich

After extra investigation, the failure in the matrix test comes from the fact that Puppi JEC uncertainties (and only the uncertainties) are missing in the 50ns GT. Should we add them in the current GT or create a new one?

( @blinkseb )

@mmusich
Copy link
Contributor

mmusich commented Apr 7, 2016

@mmarionncern, just to clarify, PUPPI JEC for 50ns do exist. Only uncertainties are not available.
As far as I can recall JETMET never provided JEC uncertainties for 50ns...
If they have suitable payload this can be included @blinkseb .
Otherwise do we still need to have the 50ns RelVals in matrix ? If yes and no palyoads for 50ns are available one can put a set of dummy ones.

@slava77
Copy link
Contributor

slava77 commented Apr 7, 2016

On 4/7/16 4:01 AM, Marco Musich wrote:

@mmarionncern https://github.com/mmarionncern, just to clarify, PUPPI
JEC for 50ns do exist. Only uncertainties are not available.
As far as I can recall JETMET never provided JEC uncertainties for 50ns...
If they have suitable payload this can be included @blinkseb
https://github.com/blinkseb .
Otherwise do we still need to have the 50ns RelVals in matrix ? If yes
and no palyoads for 50ns are available one can put a set of dummy ones.

To the first order, I'd supply the same as for 25ns.
Dummy with random numbers (like 1 or 0) may be worse.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#13954 (comment)

@mmusich
Copy link
Contributor

mmusich commented Apr 7, 2016

From a previous thread with @blinkseb:

"concerning the issue with uncertainties, it seems the database we use for 50ns does not contains the uncertainties for all the jets collection, but only for AK4PFchs. I'll update this database so that if we need another GT for 2015 the 50ns uncertainties are included for all jet collections."
I think the time has come :)
thanks,

Marco

@blinkseb
Copy link
Contributor

blinkseb commented Apr 7, 2016

Indeed ;) It's fairly easy to do, but should I request the creation of a new 50ns GT? Or maybe the new feature of creating private GT can be useful in this case?

@mmusich
Copy link
Contributor

mmusich commented Apr 7, 2016

@blinkseb go ahead and create the self-GT for this purpose! once we've seen this PR works we'll update queues and autoCond. thanks

@blinkseb
Copy link
Contributor

blinkseb commented Apr 7, 2016

Self-GT created: 80X_mcRun2_startup_Candidate_2016_04_07_12_27_27. I updated the JEC version from V5 to V6 (Summer15_50nsV6). Only changes wrt to V5 is the addition of uncertainties for all the jet algorithm.

Is there something else I need to do on my side?

@mmusich
Copy link
Contributor

mmusich commented Apr 7, 2016

@blinkseb no, thanks! @mmarionncern needs to change his version of autoCond.py in this PR and re-run the tests

@cmsbuild
Copy link
Contributor

Pull request #13954 was updated. @cvuosalo, @cerminar, @cmsbuild, @franzoni, @slava77, @mmusich, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #13954 was updated. @cvuosalo, @cerminar, @cmsbuild, @franzoni, @slava77, @mmusich, @davidlange6 can you please check and sign again.

@cvuosalo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

+1

For #13954 fc8b57c

Fixing PUPPI MET in Mini-AOD.

The code changes are satisfactory. Jenkins tests against baseline CMSSW_8_0_X_2016-04-12-2300 show only the expected differences in PUPPI MET. An extended test of workflow 25202.0_TTbar_13 with 70 events against baseline CMSSW_8_0_X_2016-04-07-2300 also shows only the expected small differences from the MET fix. RECO event content does not change, and Mini-AOD event content changes only very slightly:

   or, B         new, B      delta, B   delta, %   deltaJ, %    branch 
-----------------------------------------------------------------
    948.4 ->       950.5          2      0.2   0.00     patMETs_slimmedMETsNoHF__RECO.
-------------------------------------------------------------
    79885 ->       79888          3             0.0     ALL BRANCHES

@mmusich
Copy link
Contributor

mmusich commented Apr 15, 2016

+1

@davidlange6 davidlange6 merged commit e558d65 into cms-sw:CMSSW_8_0_X Apr 18, 2016
@slava77
Copy link
Contributor

slava77 commented Apr 19, 2016

It looks like this PR broke 50 ns run2 and run 1 workflows with PAT.
Revert or fix?

@mmusich
Copy link
Contributor

mmusich commented Apr 19, 2016

@slava77 are you referring to this:

----- Begin Fatal Exception 19-Apr-2016 06:59:27 CEST-----------------------
An exception of category 'InvalidInput' occurred while
   [0] Processing run: 172802 lumi: 41 event: 33766250
   [1] Running path 'dqmofflineOnPAT_1_step'
   [2] Calling event method for module METAnalyzer/'pfPuppiMetDQMAnalyzerMiniAOD'
   [3] Calling produce method for unscheduled module PATMETSlimmer/'slimmedMETsPuppi'
   [4] Calling produce method for unscheduled module CorrectedPATMETProducer/'patPFMetT1JetEnUpPuppi'
   [5] Calling produce method for unscheduled module ShiftedParticleMETcorrInputProducer/'shiftedPatMETCorrJetEnUpPuppi'
   [6] Calling produce method for unscheduled module ShiftedPATJetProducer/'shiftedPatJetEnUpPuppi'
Exception Message:
 cannot find key 9 in the JEC payload, this usually means you have to change the global tag

we need the 50ns uncertainty propagated everywhere we need it (i.e. both run2 50ns data and run1 data)

@mmusich
Copy link
Contributor

mmusich commented Apr 19, 2016

@blinkseb do you think it would be feasible to get the 50ns IOVs of Run2 data and Run1 data fixed adding the uncertainties?

@blinkseb
Copy link
Contributor

@mmusich Yes sure, I'm doing it right now. For Run1 data, is there an issue using for example CMSSW 8.0.X to create the database?

@mmusich
Copy link
Contributor

mmusich commented Apr 19, 2016

@blinkseb we are using the 80X_dataRun2_v* tags to cover also run1 data in autoCond. Right now looking for example at what we have proposed in GT for a random label:

https://cms-conddb.cern.ch/cmsDbBrowser/list/Prod/tags/JetCorrectorParametersCollection_Fall15_V2_DATA_AK4PF

I see:

1   2016-02-21 15:34:16,000000  bb5376a0d75485827869da3f4043f819e5f4e5a4
253000  2016-02-21 15:34:16,000000  a9c3d63fff11cac50220489fb9e11180614a835e
254833  2016-02-21 15:34:16,000000  bb5376a0d75485827869da3f4043f819e5f4e5a4
254834  2016-02-21 15:34:16,000000  a9c3d63fff11cac50220489fb9e11180614a835e
254981  2016-02-21 15:34:16,000000  bb5376a0d75485827869da3f4043f819e5f4e5a4
255032  2016-02-21 15:34:16,000000  a9c3d63fff11cac50220489fb9e11180614a835e

we need therefore a new payload for the IOV [1-25300] and also for the 50ns ones covering run2 data (I think you have the correct boundaries)

@blinkseb
Copy link
Contributor

Ok great, thanks a lot! Should I create a self-GT for this one, or request a new GT as usual via the hypernews?

@mmusich
Copy link
Contributor

mmusich commented Apr 19, 2016

please queue them and announce when done. I'll cut the new GT and make the PR, from the ORP today I understand it's kind of urgent. Thanks,
Marco

@blinkseb
Copy link
Contributor

@mmusich Announcement done: https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/2300.html

I hope it'll be ok!

@mmusich
Copy link
Contributor

mmusich commented Apr 19, 2016

@blinkseb thanks a lot: #14152
should get us rid of the failures.

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

9 participants