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

Add EP combination variables to electron Table in NANOAOD #43028

Conversation

Prasant1993
Copy link
Contributor

PR description:

This PR is to add the EP combination variables to electron table in NanoAOD. The EP combination is needed for the scales + smearing corrections for electrons below 50 GeV.
An offline code implementation to do the EP combination from NanoAOD is under progress to provide it to the analyzers.
We might need few more variables or may not in this process. This will be clear in a few days time.

PR validation:

runTheMatrix tests have been successfully run for the following workflow :

  • runTheMatrix.py -l 12434.0

The size of these variables in the event in NanoAOD is calculated according to a check with the command
python3 PhysicsTools/NanoAOD/test/inspectNanoFile.py <my_file.root> -j test.json -s test_size_report.html -d tes_doc.html

MiniAOD file used to check these additions of floats/bools in NanoAOD: /eos/cms/store/relval/CMSSW_13_3_0_pre3/RelValZEE_14/MINIAODSIM/PU_132X_mcRun3_2023_realistic_v4-v1/2580000/acbf78c0-7c57-47e9-9d72-527b46b8fc91.root

The event size before adding the variables is given here: https://prrout.web.cern.ch/prrout/Add_EP_combination_var_NanoAoD_26092023/Before_adding_variable/test_size_report.html
The event size after adding the variables is given here : https://prrout.web.cern.ch/prrout/Add_EP_combination_var_NanoAoD_26092023/After_adding_variable/test_size_report.html

Backport:

We target this to go in Nano V13 for now. Not sure yet whether backport is required to earlier CMSSW release.

Tagging EGM L2 convenors @a-kapoor and @RSalvatico

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43028/37201

  • This PR adds an extra 24KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Prasant1993 (Prasant Kumar Rout) for master.

It involves the following packages:

  • PhysicsTools/NanoAOD (xpog)

@cmsbuild, @simonepigazzini, @vlimant can you please review it and eventually sign? Thanks.
@gpetruc, @AnnikaStein this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@simonepigazzini
Copy link
Contributor

enable nano

gsfTrketaMode = Var("gsfTrack().etaMode()",float,doc="GSF track etaMode",precision=10),
gsfTrkphiMode = Var("gsfTrack().phiMode()",float,doc="GSF track phiMode",precision=10),
isEcalDriven = Var("ecalDrivenSeed",bool,doc="is ECAL driven if true"),
isEB = Var("isEB",bool,doc="object in barrel if true"),
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove isEB, this can be infered from the eta.

Copy link
Contributor Author

@Prasant1993 Prasant1993 Oct 16, 2023

Choose a reason for hiding this comment

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

Hi @simonepigazzini the decision of "isEB" is from the seed crystal and detID information. It is being used while deriving energy regression of electron by combing ecal and tracker information. We are trying to see if we can use this barrel/endcap definition from the supercluster eta of electron.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I think the difference should be negligible. It would be actually very interesting to know how many times using supercluster.eta gives a different result that the seed rec hit position (I think this question has popped up few times in the past, maybe we already know the answer ....)

Copy link
Contributor

@RSalvatico RSalvatico Oct 23, 2023

Choose a reason for hiding this comment

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

I've processed 50k electrons (slimmedElectrons in Mini, no ID, no cutoffs of sort applied) and checked how many times isEB was giving different results wrt etaSC with a barrel cutoff at |eta| = 1.479. I found that only one electron had |eta| > 1.479 (slightly) when isEB == true. That makes it a 0.002% difference. My opinion is that we could go with etaSC and accept that there will be a difference of this order of magnitude wrt the correction done on the same events in MiniAOD.

Note that, if the barrel eta cutoff changes slightly (e.g., 1.4442, which is the number we typically use as the start of the calorimeter transition region), these numbers become dramatically different and the discrepancy is rather a 2.5% one.

@simonepigazzini
Copy link
Contributor

type egamma

@simonepigazzini
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-35e494/35207/summary.html
COMMIT: 27453f8
CMSSW: CMSSW_13_3_X_2023-10-16-1100/el8_amd64_gcc12
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43028/35207/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 161 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3356920
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3356892
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 214 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 15925
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 15925
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 14 files compared)
  • Checked 34 log files, 16 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.0 2.501 2.473 0.028 ( +1.1% ) 5.36 5.28 +1.4% 2.126 2.128
2500.001 2.646 2.616 0.031 ( +1.2% ) 4.75 4.71 +0.9% 2.539 2.542
2500.002 2.554 2.526 0.028 ( +1.1% ) 4.96 4.90 +1.3% 2.545 2.542
2500.01 1.285 1.265 0.020 ( +1.6% ) 9.87 9.72 +1.6% 2.247 2.237
2500.011 1.661 1.636 0.025 ( +1.5% ) 5.35 5.25 +1.9% 2.406 2.408
2500.012 1.545 1.519 0.026 ( +1.7% ) 7.66 7.54 +1.6% 2.333 2.341
2500.1 2.153 2.127 0.026 ( +1.2% ) 5.41 5.32 +1.6% 1.988 1.980
2500.2 2.263 2.238 0.025 ( +1.1% ) 6.17 6.09 +1.4% 1.892 1.893
2500.21 1.148 1.125 0.023 ( +2.1% ) 4.38 4.35 +0.7% 2.175 2.180
2500.211 1.506 1.480 0.026 ( +1.7% ) 3.85 3.77 +2.2% 2.256 2.260
2500.3 2.023 1.995 0.027 ( +1.4% ) 12.93 12.57 +2.8% 1.883 1.883
2500.31 1.224 1.202 0.022 ( +1.9% ) 20.41 19.69 +3.7% 2.260 2.274
2500.311 1.606 1.581 0.025 ( +1.6% ) 14.08 13.53 +4.1% 2.323 2.349
2500.4 2.023 1.995 0.027 ( +1.4% ) 12.94 12.53 +3.2% 1.885 1.880
2500.5 19.544 19.544 0.000 ( +0.0% ) 1.39 1.27 +9.0% 1.301 1.292

@vlimant
Copy link
Contributor

vlimant commented Oct 25, 2023

please add the relevant plots in the nano dqm

@Prasant1993
Copy link
Contributor Author

please add the relevant plots in the nano dqm

Okay. I will add them in the nano DQM.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43028/37365

  • This PR adds an extra 32KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

Pull request #43028 was updated. @cmsbuild, @vlimant, @simonepigazzini can you please check and sign again.

@simonepigazzini
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-35e494/35415/summary.html
COMMIT: 278ac03
CMSSW: CMSSW_13_3_X_2023-10-24-2300/el8_amd64_gcc12
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43028/35415/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 71 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 173 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3351440
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3351412
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 72.76400000000001 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 7.540 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 13234.0,... ): 4.996 KiB Physics/NanoAODDQM
  • Checked 213 log files, 166 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • You potentially added 2 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 15904
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 15904
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 87.75200000000001 KiB( 14 files compared)
  • DQMHistoSizes: changed ( 2500.001,... ): 7.540 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 2500.011,... ): 4.996 KiB Physics/NanoAODDQM
  • Checked 34 log files, 16 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.0 2.531 2.500 0.030 ( +1.2% ) 5.18 5.25 -1.3% 2.115 1.998
2500.001 2.677 2.649 0.028 ( +1.1% ) 4.81 4.69 +2.5% 2.127 2.387
2500.002 2.611 2.556 0.054 ( +2.1% ) 4.95 4.87 +1.6% 2.145 2.373
2500.01 1.304 1.285 0.019 ( +1.5% ) 10.03 9.58 +4.7% 2.213 2.095
2500.011 1.717 1.659 0.058 ( +3.5% ) 5.31 5.21 +2.1% 2.015 2.225
2500.012 1.564 1.539 0.025 ( +1.6% ) 7.71 7.47 +3.3% 2.028 2.141
2500.1 2.183 2.155 0.028 ( +1.3% ) 5.34 5.30 +0.6% 1.965 1.865
2500.2 2.294 2.266 0.027 ( +1.2% ) 6.19 6.07 +2.1% 1.846 1.756
2500.21 1.170 1.147 0.023 ( +2.0% ) 4.43 4.33 +2.3% 1.875 2.108
2500.211 1.531 1.505 0.026 ( +1.7% ) 3.91 3.81 +2.6% 1.972 2.085
2500.3 2.048 2.023 0.025 ( +1.2% ) 12.67 12.63 +0.3% 1.880 1.775
2500.31 1.245 1.223 0.022 ( +1.8% ) 20.62 19.98 +3.2% 2.282 2.107
2500.311 1.631 1.606 0.025 ( +1.6% ) 14.34 13.43 +6.8% 2.361 2.152
2500.4 2.048 2.023 0.025 ( +1.2% ) 12.97 12.73 +1.8% 1.878 1.768
2500.5 19.544 19.544 0.000 ( +0.0% ) 0.94 0.99 -4.6% 1.296 1.288

@Prasant1993
Copy link
Contributor Author

Hi @simonepigazzini and @vlimant , should we go ahead with this PR or any further changes are needed ?

@simonepigazzini
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 2, 2023

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @antoniovilela, @sextonkennedy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@rappoccio
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 4a76809 into cms-sw:master Nov 3, 2023
@RSalvatico RSalvatico mentioned this pull request Feb 13, 2024
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.

7 participants