backport tau pog nanoAOD clean up to 10_6_X #33658

swozniewski · 2021-05-07T12:29:37Z

PR description:

clean up of tau related nanoAOD content and DQM, i.e. removal of deprecated MVA IDs, update of decayModeID flags and according update of selection.
changes are supposed to leave run2_nanoAOD_106Xv1 and earlier nanoAOD untouched.

PR validation:

passed code checks/format, unit tests, limited matrix tests

if this PR is a backport please specify the original PR and why you need to backport that PR:

backport of #33513 to CMSSW 10_6_X for nanoAODv9 producetion.

cmsbuild · 2021-05-07T12:30:00Z

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

It involves the following packages:

PhysicsTools/NanoAOD

@cmsbuild, @mariadalfonso, @gouskos, @fgolf can you please review it and eventually sign? Thanks.
@gpetruc, @swertz 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 · 2021-05-07T12:53:32Z

please test

cmsbuild · 2021-05-07T15:53:43Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-933c25/14940/summary.html
COMMIT: 5b595f1
CMSSW: CMSSW_10_6_X_2021-05-07-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33658/14940/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
Reco comparison results: 87 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215540
DQMHistoTests: Total failures: 30
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215176
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -4.478 KiB( 34 files compared)
DQMHistoSizes: changed ( 136.8523 ): -4.478 KiB Physics/NanoAODDQM
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

mariadalfonso · 2021-05-10T08:35:39Z

PhysicsTools/NanoAOD/python/nanoDQM_cff.py

+from Configuration.Eras.Modifier_run2_nanoAOD_92X_cff import run2_nanoAOD_92X
+from Configuration.Eras.Modifier_run2_nanoAOD_94XMiniAODv1_cff import run2_nanoAOD_94XMiniAODv1
+from Configuration.Eras.Modifier_run2_nanoAOD_94XMiniAODv2_cff import run2_nanoAOD_94XMiniAODv2
+from Configuration.Eras.Modifier_run2_nanoAOD_94X2016_cff import run2_nanoAOD_94X2016
+from Configuration.Eras.Modifier_run2_nanoAOD_102Xv1_cff import run2_nanoAOD_102Xv1
+from Configuration.Eras.Modifier_run2_nanoAOD_106Xv1_cff import run2_nanoAOD_106Xv1


you can profit to replace all these import with
from PhysicsTools.NanoAOD.nano_eras_cff import *
(to add at the beginning of the file)

mariadalfonso · 2021-05-10T08:38:13Z

PhysicsTools/NanoAOD/python/taus_cff.py

@@ -139,13 +142,13 @@ def _tauId8WPMask(pattern,doc):

 _variablesMiniV2 = cms.PSet(
    _tauVarsBase,
-    _mvaAntiEVars,


The _deepTauVars2017v2 is not present in master and is also unused in 10_6, so can be cleaned up

https://github.com/cms-sw/cmssw/pull/33658/files#diff-d54676262d2e5326ee3455e57747fd476b12be16993a8eb0f4794e8771f6526fL123-L129

mariadalfonso · 2021-05-10T08:40:21Z

PhysicsTools/NanoAOD/python/taus_cff.py

@@ -158,25 +161,29 @@ def _tauId8WPMask(pattern,doc):

 tauTable.variables = _variablesMiniV2

+(run2_nanoAOD_92X | run2_nanoAOD_94XMiniAODv2 | run2_nanoAOD_94X2016 | run2_nanoAOD_102Xv1 | run2_nanoAOD_106Xv1).toModify(tauTable,
+                 variables = cms.PSet(tauTable.variables, _mvaAntiEVars, _mvaIsoVars2015Reduced, _mvaIsoVars2017v1, _mvaIsoVars2017v2)


we removed the _mvaAntiEVars in the master, why not here as well ?

I don't know why, but the era dependency of _mvaAntiEVars here was arranged the other way round than in master, i.e. the default was what is needed in previous eras and for 106v2 it was changed via modifier. I was conservative with my change and only removed the modifier, but if you prefer, I can arrange it like master.

Just for completeness: Concerning the line above in particular, this required to restore mvaAntiEVars since it is not done by a following modifier.

mariadalfonso · 2021-05-10T08:46:32Z

PhysicsTools/NanoAOD/python/taus_cff.py

-                            rawAntiEle = Var("tauID('againstElectronMVA6Raw2015')", float, doc= "Anti-electron MVA discriminator V6 raw output discriminator (2015)", precision=10),
-                            rawAntiEleCat = Var("tauID('againstElectronMVA6category2015')", int, doc="Anti-electron MVA discriminator V6 category (2015"),
-                            idAntiEle = _tauId5WPMask("againstElectron%sMVA62015", doc= "Anti-electron MVA discriminator V6 (2015)")
+                            idAntiEleDeadECal = Var("tauID('againstElectronDeadECAL')", bool, doc = "Anti-electron dead-ECal discriminator")


this should be done like in master:

one set the future as default, and then rewire the past.

https://github.com/cms-tau-pog/cmssw/blob/9aef44e5dbcc067ab65d21177b3ef40426426d52/PhysicsTools/NanoAOD/python/taus_cff.py#L81

https://github.com/cms-tau-pog/cmssw/blob/9aef44e5dbcc067ab65d21177b3ef40426426d52/PhysicsTools/NanoAOD/python/taus_cff.py#L176-L179

alright, to be updated. As I said above, I tried to keep changes wrt. present implementation small. But in fact I was wondering why this was not consistent with the convention you are describing.

I will adapt the logic for idAntiEleDeadECal as well (while the quantity itself was not part of the clean up so far)

cmsbuild · 2021-05-10T13:38:33Z

Pull request #33658 was updated. @cmsbuild, @mariadalfonso, @gouskos, @fgolf can you please check and sign again.

swozniewski · 2021-05-10T13:43:44Z

@mariadalfonso I applied the requested changes.

mariadalfonso · 2021-05-10T14:15:28Z

please test

mariadalfonso · 2021-05-11T16:15:46Z

Thanks a lot for this investigation !
Now we understand why the backport wasn't 1:1.
I think we can revert this againstElectronDeadECALForNano vs againstElectronDeadECAL as was originally.
Sorry for more work.

Also worth to add a comment in the python so that we remember later.

Would we have noticed before if sth. was broken within 10_6_X for the modifiers run2_nanoAOD_106Xv2 and run2_miniAOD_devel? I have the impression that these could not work because the following commit was not backported to 10_6_X: e6bfb8f
The given test without nanoAOD era modifier was treated like previous eras in the way it is written here: https://github.com/cms-sw/cmssw/pull/33658/files#diff-d54676262d2e5326ee3455e57747fd476b12be16993a8eb0f4794e8771f6526fL172
Only for the two named modifiers, the missing quantity was (would have been) accessed. But now it is the default

swozniewski · 2021-05-11T17:05:30Z

I hope that reverting will also work for run2_nanoAOD_106Xv2 with nanoAODv9 (not sure whether this is included in the tests). But I think latest UL miniAOD contains the quantity due to these lines https://github.com/cms-sw/cmssw/blob/CMSSW_10_6_X/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L414-L430
Therefore it should be fine. It is restricted to run2_miniAOD_UL, which I think explains why the test failed.
I'll revert the era handling of againstElectronDeadECAL...

cmsbuild · 2021-05-11T17:31:52Z

Pull request #33658 was updated. @cmsbuild, @mariadalfonso, @gouskos, @fgolf can you please check and sign again.

mariadalfonso · 2021-05-11T18:04:05Z

please test

mariadalfonso · 2021-05-11T18:07:34Z

I hope that reverting will also work for run2_nanoAOD_106Xv2 with nanoAODv9 (not sure whether this is included in the tests). But I think latest UL miniAOD contains the quantity due to these lines https://github.com/cms-sw/cmssw/blob/CMSSW_10_6_X/PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py#L414-L430
Therefore it should be fine. It is restricted to run2_miniAOD_UL, which I think explains why the test failed.
I'll revert the era handling of againstElectronDeadECAL...

yes, cmsbuild tests the run2_nanoAOD_106Xv2 here, this is the wf 136.8523
and the nanobot test all the modifiers available in any release :)

cmsbuild · 2021-05-11T20:57:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-933c25/15006/summary.html
COMMIT: b4d88c0
CMSSW: CMSSW_10_6_X_2021-05-11-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33658/15006/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
Reco comparison results: 95 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215540
DQMHistoTests: Total failures: 32
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3215174
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -4.478 KiB( 34 files compared)
DQMHistoSizes: changed ( 136.8523 ): -4.478 KiB Physics/NanoAODDQM
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

mariadalfonso · 2021-05-13T08:34:36Z

please test

(want to recheck with a more update IB where similar changes are made on nanoDQM_cff.py)

cmsbuild · 2021-05-13T12:44:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-933c25/15058/summary.html
COMMIT: b4d88c0
CMSSW: CMSSW_10_6_X_2021-05-12-2300/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33658/15058/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

No significant changes to the logs found
Reco comparison results: 1318 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 3215568
DQMHistoTests: Total failures: 26505
DQMHistoTests: Total nulls: 22
DQMHistoTests: Total successes: 3188707
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 52.227 KiB( 34 files compared)
DQMHistoSizes: changed ( 136.8523 ): -4.478 KiB Physics/NanoAODDQM
DQMHistoSizes: changed ( 140.53 ): 44.531 KiB Hcal/DigiRunHarvesting
DQMHistoSizes: changed ( 140.53 ): 10.938 KiB Info/EventInfo
DQMHistoSizes: changed ( 140.53 ): 1.172 KiB RPC/DCSInfo
DQMHistoSizes: changed ( 140.53 ): 0.064 KiB SiStrip/MechanicalView
Checked 143 log files, 29 edm output root files, 35 DQM output files
TriggerResults: no differences found

mariadalfonso · 2021-05-14T14:49:30Z

+xpog
changes inline with master and follow up review
(most of the "1318 differences found in the comparisons" are due to the wf 140.53 and unrelated to this PR)

cmsbuild · 2021-05-14T14:49:52Z

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_12_0_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)

qliphy · 2021-05-15T05:35:12Z

+1

Sebastian Wozniewski added 8 commits April 27, 2021 10:34

clean up tau nanoAOD content

2cb79cb

fix era modification

d28a422

avoid parameter collisions in temporary PSets

a26149f

restore MVA2017v2 for 94XMiniAODv1 and further clean up

52d8d91

clean up DQM

3acf64b

restore tau DQM for previous eras

94cceac

fix syntax of removal of idDecayModeOldDMs in DQM

6714a54

import missing era modifiers to DQM config

5b595f1

cmsbuild added this to the CMSSW_10_6_X milestone May 7, 2021

cmsbuild added orp-pending pending-signatures tests-pending xpog-pending labels May 7, 2021

cmsbuild added tests-started and removed tests-pending labels May 7, 2021

mariadalfonso mentioned this pull request May 7, 2021

features for V9 nanoAOD cms-nanoAOD/cmssw#555

Closed

55 tasks

cmsbuild added tests-approved and removed tests-started labels May 7, 2021

mariadalfonso reviewed May 10, 2021

View reviewed changes

follow up review

3b57fbf

cmsbuild added tests-pending and removed tests-approved labels May 10, 2021

cmsbuild added tests-started and removed tests-pending labels May 10, 2021

revert changes of againstElectronDeadECAL readout

b4d88c0

cmsbuild added tests-pending and removed tests-rejected labels May 11, 2021

cmsbuild added tests-started and removed tests-pending labels May 11, 2021

cmsbuild added tests-approved and removed tests-started labels May 11, 2021

cmsbuild added tests-started and removed tests-approved labels May 13, 2021

cmsbuild added tests-approved and removed tests-started labels May 13, 2021

cmsbuild added fully-signed xpog-approved and removed pending-signatures xpog-pending labels May 14, 2021

cmsbuild added orp-approved and removed orp-pending labels May 15, 2021

cmsbuild merged commit f4300df into cms-sw:CMSSW_10_6_X May 15, 2021

mbluj deleted the CMSSW_10_6_X_backport_CMSSW_11_3_X_tau-pog_nanoAODcleanUp branch October 10, 2023 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backport tau pog nanoAOD clean up to 10_6_X #33658

backport tau pog nanoAOD clean up to 10_6_X #33658

swozniewski commented May 7, 2021

cmsbuild commented May 7, 2021

mariadalfonso commented May 7, 2021

cmsbuild commented May 7, 2021

mariadalfonso May 10, 2021

mariadalfonso May 10, 2021

mariadalfonso May 10, 2021

swozniewski May 10, 2021

swozniewski May 10, 2021

mariadalfonso May 10, 2021

swozniewski May 10, 2021

swozniewski May 10, 2021

cmsbuild commented May 10, 2021

swozniewski commented May 10, 2021

mariadalfonso commented May 10, 2021

mariadalfonso commented May 11, 2021 •

edited

swozniewski commented May 11, 2021

cmsbuild commented May 11, 2021

mariadalfonso commented May 11, 2021

mariadalfonso commented May 11, 2021

cmsbuild commented May 11, 2021

mariadalfonso commented May 13, 2021

cmsbuild commented May 13, 2021

mariadalfonso commented May 14, 2021 •

edited

cmsbuild commented May 14, 2021

qliphy commented May 15, 2021

backport tau pog nanoAOD clean up to 10_6_X #33658

backport tau pog nanoAOD clean up to 10_6_X #33658

Conversation

swozniewski commented May 7, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented May 7, 2021

mariadalfonso commented May 7, 2021

cmsbuild commented May 7, 2021

Comparison Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented May 10, 2021

swozniewski commented May 10, 2021

mariadalfonso commented May 10, 2021

mariadalfonso commented May 11, 2021 • edited

swozniewski commented May 11, 2021

cmsbuild commented May 11, 2021

mariadalfonso commented May 11, 2021

mariadalfonso commented May 11, 2021

cmsbuild commented May 11, 2021

Comparison Summary

mariadalfonso commented May 13, 2021

cmsbuild commented May 13, 2021

Comparison Summary

mariadalfonso commented May 14, 2021 • edited

cmsbuild commented May 14, 2021

qliphy commented May 15, 2021

mariadalfonso commented May 11, 2021 •

edited

mariadalfonso commented May 14, 2021 •

edited