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 for Puppi MET and MET significance compatibility between AOD and miniAOD (80X), rebased #18341

Closed

Conversation

mmarionncern
Copy link

Previous version was #16174, rebased on top of 80X.
Planned for 80X miniAODv2

Packages touched :

  • PhysicsTools/PatAlgos
  • PhysicsTools/PatUtils
  • RecoMET/METAlgorithms
  • RecoMET/METProducers

Changes :

  • fix scheduled mode (do not affect unscheduled mode)
  • change the constructor of MET in the RecoMET extractor to copy properly the sumET from the slimmedMET collection when updating a miniAOD
  • MET significance computation at the AOD level updated to match the miniAOD level
  • Update of the test cfg files
  • fix the puppi JEC and the use of slimmedJetPuppi when recomputing the puppiMET from miniAOD
  • modify the MET signifiance code to match by reference the pfCandidates to the jets/lepton constituents + use of an extra recovery by delta"4Vector" matching when reference matching is failing

Expected changes in data file contents :

  • small changes in the mET signifiance when producing miniAOD
  • puppi MET values when rerunning on miniAOD
  • storage of the sumEt when recomputing MET on top of miniAOD

@cmsbuild
Copy link
Contributor

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

@slava77
Copy link
Contributor

slava77 commented May 11, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 11, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19755/console Started: 2017/05/11 14:23

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • You potentially added 5 lines to the logs
  • Reco comparison results: 23 differences found in the comparisons
  • DQMHistoTests: Total files compared: 16
  • DQMHistoTests: Total histograms compared: 1164335
  • DQMHistoTests: Total failures: 1374
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1162843
  • DQMHistoTests: Total skipped: 118
  • DQMHistoTests: Total Missing objects: 0
  • Checked 63 log files, 14 edm output root files, 16 DQM output files

@slava77
Copy link
Contributor

slava77 commented May 15, 2017

+1

for #18341 92f73cd

  • updates to miniAOD MET variables: modified regular MET significance, changed computation of PUPPI MET. All are in the scope of legacy re-miniAOD v2 (in case it happens in 80X). Changes files are consistent with what we have in 9X/master.
  • jenkins tests pass and comparisons with baseline show differences in PUPPI MET (all variables) and in slimmedMET significance. All are expected.

I will put this PR on hold to indicate that it is supposed to be merged only when we are ready to build a re-miniAOD v2 release.

@slava77
Copy link
Contributor

slava77 commented May 15, 2017

hold

to indicate that it is supposed to be merged only when we are ready to build a re-miniAOD v2 release.

@cmsbuild
Copy link
Contributor

Pull request has been put on hold by @slava77
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

@ahinzmann
Copy link
Contributor

@slava77 We'd like to backport also #19587 for 80X. Should we merge it into this PR, or what is the appropriate way to get this into 80X?

@slava77
Copy link
Contributor

slava77 commented Aug 2, 2017 via email

@zdemirag
Copy link
Contributor

zdemirag commented Aug 3, 2017

@slava77 : I am not sure I follow your comment on that it is possible this PR will never be merged in 80X. The statement was "to indicate that it is supposed to be merged only when we are ready to build a re-miniAOD v2 release."

These changes are needed for the legacy re-reco, re-miniaod v2 release.

@slava77
Copy link
Contributor

slava77 commented Aug 3, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Jan 30, 2018

@mmarionncern @zdemirag
The plan is fairly clear now to not make re-miniAOD of the legacy 80X/2016 with 80X.

Please close this PR or clarify the reasons why it should stay open.

Thank you.

@gouskos
Copy link
Contributor

gouskos commented Feb 1, 2018

Hi @slava77, This means that fixes in this are not included in the 9X versions? Thanks.

@mmarionncern
Copy link
Author

If 80X is not used, the PR can be closed

@slava77
Copy link
Contributor

slava77 commented Feb 1, 2018 via email

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