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

UL2016 Scale and smearing corrections (backport to 106X) #33359

Merged
merged 4 commits into from Apr 9, 2021

Conversation

jainshilpi
Copy link
Contributor

PR description:

This PR is essentially a backport of #33030 and adds UL 2016 scale and smearing corrections in miniAOD in 106X.
Backport concerning nanoAOD will be done in a separate PR.

Changes are done only to RecoEgamma/EgammaTools/python/calibratedEgammas_cff.py

@mariadalfonso @sroychow

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2021

A new Pull Request was created by @jainshilpi for CMSSW_10_6_X.

It involves the following packages:

RecoEgamma/EgammaTools

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @lgray, @sobhatta, @afiqaize, @varuns23, @ram1123 this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@mariadalfonso
Copy link
Contributor

mariadalfonso commented Apr 7, 2021

@jainshilpi
this mini change should go for the b-parking reMINI only as the run2_miniAOD_UL is bounded to the existing pp ongoing production

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2021

Pull request #33359 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

@@ -14,6 +16,11 @@
)
from Configuration.ProcessModifiers.run2_miniAOD_UL_cff import run2_miniAOD_UL

from Configuration.Eras.Modifier_run2_egamma_2016_cff import run2_egamma_2016
from Configuration.Eras.Modifier_tracker_apv_vfp30_2016_cff import tracker_apv_vfp30_2016
(run2_egamma_2016 & tracker_apv_vfp30_2016).toModify(calibratedEgammaSettings,correctionFile = _correctionFile2016ULpreVFP)
Copy link
Contributor

Choose a reason for hiding this comment

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

this will still get applied with run2_miniAOD_UL and without it.
I think that an applicable requirement here is run2_miniAOD_devel

Suggested change
(run2_egamma_2016 & tracker_apv_vfp30_2016).toModify(calibratedEgammaSettings,correctionFile = _correctionFile2016ULpreVFP)
(run2_miniAOD_devel & run2_egamma_2016 & tracker_apv_vfp30_2016).toModify(calibratedEgammaSettings,correctionFile = _correctionFile2016ULpreVFP)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay then I will change this in both the 2016 UL modifiers

Copy link
Contributor

Choose a reason for hiding this comment

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

@slava77
for my own understanding the bParking will inherit all we had in the run2_miniAOD_devel or run2_miniAOD_UL

Copy link
Contributor

Choose a reason for hiding this comment

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

bParking does not apply to 2016.
If this was for 2018, then we'd use (run2_miniAOD_devel | bParking)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

missed to include the cff file - sorry about it - done now

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2021

Pull request #33359 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Apr 7, 2021

@cmsbuild please test workflow 136.72411,136.76111,136.77211,136.83111,136.88811,1325.516,1325.5161,1325.517,1325.518

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2021

-1

Failed Tests: UnitTests RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a86311/14079/summary.html
COMMIT: cfbcadd
CMSSW: CMSSW_10_6_X_2021-04-07-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33359/14079/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test runtestPhysicsToolsNanoAOD had ERRORS

RelVals

  • 136.7611136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log
  • 136.72411136.72411_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM/step2_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM.log
  • 136.76111136.76111_RunJetHT2016E_reminiaodUL+RunJetHT2016E_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM/step2_RunJetHT2016E_reminiaodUL+RunJetHT2016E_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM.log
Expand to see more relval errors ...

RelVals-INPUT

  • 4.64.6_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1/step2_MinimumBias2010A+MinimumBias2010A+RECOSKIMALCA+HARVESTDR1.log
  • 136.72411136.72411_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM/step2_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM.log
  • 136.76111136.76111_RunJetHT2016E_reminiaodUL+RunJetHT2016E_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM/step2_RunJetHT2016E_reminiaodUL+RunJetHT2016E_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM.log
Expand to see more relval errors ...

@@ -1,9 +1,11 @@
import FWCore.ParameterSet.Config as cms

Copy link
Contributor

Choose a reason for hiding this comment

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

@jainshilpi you need to add
from Configuration.Eras.Modifier_run2_miniAOD_devel_cff import run2_miniAOD_devel

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2021

Pull request #33359 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Apr 7, 2021

@cmsbuild please test workflow 136.72411,136.76111,136.77211,136.83111,136.88811,1325.516,1325.5161,1325.517,1325.518

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a86311/14080/summary.html
COMMIT: 4ddfc2b
CMSSW: CMSSW_10_6_X_2021-04-07-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33359/14080/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-a86311/136.72411_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 3215541
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3215205
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
  • Checked 143 log files, 29 edm output root files, 35 DQM output files
  • TriggerResults: no differences found

@slava77
Copy link
Contributor

slava77 commented Apr 8, 2021

+reconstruction

for #33359 4ddfc2b

  • code changes are in line with the PR description and the follow up review
  • jenkins tests pass and comparisons with the baseline show no (relevant) differences

cms-sw/cmsdist#6800 is needed with this PR (async merge is OK)

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2021

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_3_X is complete. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 769dffb into cms-sw:CMSSW_10_6_X Apr 9, 2021
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

6 participants