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 MuEGCleaning input collections #17395

Merged
merged 1 commit into from Feb 3, 2017

Conversation

mmarionncern
Copy link

Fix the input EGamma collections used to compute the EG fix for the re-miniAOD campaign.
One can still derive the fully corrected MET from what is inside the miniAODs, but that would fix the MuEG collection

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2017

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

It involves the following packages:

PhysicsTools/PatAlgos

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

cms-bot commands are listed here #13028

@gpetruc
Copy link
Contributor

gpetruc commented Feb 2, 2017

Should we make a patch?
It can also be fixed in --customize_commands, though it's not a good approach.
The other option is to ignore this, and tell people to do it right at analysis level.

@slava77
Copy link
Contributor

slava77 commented Feb 2, 2017 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17596/console Started: 2017/02/02 19:19

@slava77
Copy link
Contributor

slava77 commented Feb 2, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Feb 2, 2017 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2017

@davidlange6
Copy link
Contributor

a new record for the time between a talk announcing all was done and the first bug?

@davidlange6
Copy link
Contributor

can make a patch tomorrow once @slava77 approves if its useful..

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2017

local test with /DoubleEG/sharper-Run2016E-23Sep2016-v1_AOD_DiHEEPWOSS_GainSwitch shows differences only in slimmedMETsMuEGClean (From slides yesterday: slimmedMETsMuEGClean is supposed to be corrected for muons and for the ecal gain switch on e/γobjects [ this is likely to be the recommended one to use ]). ).

slimmedMETsMuEGClean.pt is much smaller in some events
patmets_slimmedmetsmuegclean__pat obj pt delta
patmets_slimmedmetsmuegclean__pat obj pt

which is suggestive of an improvement

Still, there is something odd:
slimmedMETsMuEGClean.sumEt is now smaller
patmets_slimmedmetsmuegclean__pat obj sumet delta

Looking at the changes in more detail: the old version was using identical inputs to corMETElectronMuEGClean and corMETEPhotonsMuEGClean which would mean they were 0; the fix now, apparently, makes these corrections equal to the correction size of the egm objects.
However, the puzzling part is the reduction in sumET. Shouldn't it be going up?
Is there a bug in https://github.com/cms-sw/cmssw/blob/CMSSW_9_0_X/PhysicsTools/PatUtils/plugins/ShiftedParticleMETcorrInputProducer.cc or am I missing something?
[if the sumEt in the shifted is buggy, I guess we are supposed to live with it in 80X because the code is 5 years old]

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2017

+1

for #17395 56f062b

  • pick appropriate base egamma collections to compute corrections for slimmedMETsMuEGClean
  • jenkins tests pass and comparisons with baseline show no differences (none expected given that this change affects only the customized PAT)
  • local checks show changes only in slimmedMETsMuEGClean, see details in Fix MuEGCleaning input collections #17395 (comment) [the effect of the fix as seen in expanded config appears to be logically correct]

@mmarionncern
Copy link
Author

@slava77 Without that fi the MuEG MET does nto receive the EGamma corrections, and one would have to extract it from the other METs and reapply it on top of the Mu corrected MET. So it is not vital, but preferable

@gpetruc
Copy link
Contributor

gpetruc commented Feb 3, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2017

On 2/3/17 8:35 AM, Giovanni Petrucciani wrote:

Looking at some events, the correction to the met vector seems to have the
correct sign, which is good.

From the code, sumEt looks indeed computed with the wrong sign but i agree
that we can live with it, it's the px and py that really matter.

@mmarionncern , please submit a fix for ShiftedParticleMETcorrInputProducer.cc to 90X or direct to someone
else to follow it @zdemirag @gouskos
it may be worth to check that it hasn't propagated to other similar MET correction algorithms

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2017 via email

@mmarionncern
Copy link
Author

@slava77
the ShiftedParticleMETcorrInputProducer class is corrected for 90X in :
#17405

@davidlange6
Copy link
Contributor

I guess its <4 hours - but should we go ahead now and then re-declare that all is done and ready for production with the patch in place?

@davidlange6
Copy link
Contributor

(or should we wait a while longer?)

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2017 via email

@davidlange6
Copy link
Contributor

davidlange6 commented Feb 3, 2017 via email

@gpetruc
Copy link
Contributor

gpetruc commented Feb 3, 2017 via email

@davidlange6 davidlange6 merged commit 2ce1d9f into cms-sw:CMSSW_8_0_X Feb 3, 2017
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

5 participants