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

Muon+JetMET part of the re-miniAOD fix #17308

Merged
merged 100 commits into from Feb 1, 2017

Conversation

@gpetruc
Copy link
Contributor

gpetruc commented Jan 27, 2017

PR that extends #17296 implementing the muon + jetmet side of the fixes.

The code can be activated, together with the E/gamma one, with

from PhysicsTools.PatAlgos.slimming.customizeMiniAOD_MuEGFixMoriond2017 import customizeAll
process = customizeAll(process)

All the core functionality is there, but we may have some updates later to try reduce the amount of modules that are run (now there's quite some duplication) and understand the performance.
We put the PR anyway today so that the review of most of the code can already start before the sun sets on the west coast.

An AOD input file for testing with about 15 events with bad muons and 15 with gain switches is in /afs/cern.ch/work/g/gpetrucc/micro/CMSSW_8_0_25/src/horrorCollection.root

@mmarionncern @rafaellopesdesa @Sam-Harper @arizzi

@slava77

This comment has been minimized.

Copy link
Contributor

slava77 commented Feb 1, 2017

wf136 731_slimmedmetnohf

still differences

@slava77

This comment has been minimized.

Copy link
Contributor

slava77 commented Feb 1, 2017

in fact, I'm getting the same default PAT expanded config with efba37c and with 30e4996.

@mmarionncern

This comment has been minimized.

Copy link

mmarionncern commented Feb 1, 2017

@slava77 @gpetruc
Corrected in
gpetruc#12
An initialization was not done properly

Fix noHF MET, second step
@cmsbuild

This comment has been minimized.

Copy link
Contributor

cmsbuild commented Feb 1, 2017

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

@slava77

This comment has been minimized.

Copy link
Contributor

slava77 commented Feb 1, 2017

@cmsbuild please test

@cmsbuild

This comment has been minimized.

Copy link
Contributor

cmsbuild commented Feb 1, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17563/console Started: 2017/02/01 14:07

@cmsbuild cmsbuild added tests-started and removed tests-pending labels Feb 1, 2017
@slava77

This comment has been minimized.

Copy link
Contributor

slava77 commented Feb 1, 2017

expanded configs look promising

@cmsbuild

This comment has been minimized.

Copy link
Contributor

cmsbuild commented Feb 1, 2017

Comparison job queued.

@slava77

This comment has been minimized.

Copy link
Contributor

slava77 commented Feb 1, 2017

+1

for #17308 4d1fca5

adding to comments in #17308 (comment), noting changes since ce5fa91

  • fixes were added to recover consistent picture in NoHF MET computation in miniAOD
  • jenkins tests passed .. but comparisons are not out yet (the job haven't started yet an hour after being queued)
  • local tests (2016E data PDs, 2K events) with the customizeAll added show that the CPU time has increase by ~50ms more ( 900 ms/ev from the total of this PR) and there are some changes in the NoHF MET since the version in ce5fa91 [assuming that's correct]
  • check of expanded config of the default PAT setup shows consistent configuration between the baseline and this PR with some changes in names of the modules related to NoHF MET and jet
  • local tests with default PAT config show that there are no more changes in NoHF MET plots
@davidlange6 davidlange6 merged commit 648ef2e into cms-sw:CMSSW_8_0_X Feb 1, 2017
1 check passed
1 check passed
default Tests OK
Details
@cmsbuild

This comment has been minimized.

@slava77

This comment has been minimized.

Copy link
Contributor

slava77 commented Feb 2, 2017

looking at jenkins comparisons, it looks like my local test observations are confirmed and there are not changes in the (monitored) defaults

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.