Add tauID against electrons in dead ECal regions (106X)

mbluj commented 4 years ago

PR description:

It is about adding a tauID against electrons in dead-ECal regions - one boolean in tauTable as mentioned in https://github.com/cms-nanoAOD/cmssw/issues/519 issue.

We note that the PR contains also a lot of unrelated stuff which should disappear when cms-nanoAOD/master-106X branch is rebased to the current state of official CMSSW_10_6_X one.

We also plan sibling PRs to branches corresponding with 10_2_X and current master CMSSW (11_2_X):

The PR to 10_2_X is submitted #522; it requires update of cms-nanoAOD/master-102X branch to match official CMSSW_10_2_X one, see comment therein;
The PR to 11_2_X can be done already now, but we note that cms-nanoAOD/master-cmsswmaster is quite behind (~2.5k commits) after official master, so we prefer to wait until they are synchronized. The PR to 11_2_X can be also considered as PR to 111 as the two do not differ nowadays. Content of 11_2/1_X PR will be identical to current (10_6_X) one.

PR validation:

Validated with running nanoAOD configuration on an MiniAOD sample obtained with 10_6_12 with cms-sw#29739 topic merged in (as in current CMSSW_10_6_X branch). The newly added variable appears unless an modifier for an existing nanoAOD related era is specified.

mariadalfonso commented 4 years ago

@mbluj we are updating the master of 10_6 https://github.com/cms-nanoAOD/cmssw/pull/529

Once done I will ask you to rebase so that the pr20739 is removed from the list

mbluj commented 4 years ago

@mbluj we are updating the master of 10_6 #529

Once done I will ask you to rebase so that the pr20739 is removed from the list

Thanks and sure I will rebase. But please note that I take two weeks off since this Saturday, so it will be better if it is ready by end of this week to converge quickly.

mariadalfonso commented 4 years ago

@mbluj the 10_6 branch is updated to 10_6_14.

can you rebase this PR ?

mbluj commented 4 years ago

Rebased to cms-nanoAOD:master-106X so that only two files related to inclusion of new tauID are modified by this PR.

gpetruc-bot commented 4 years ago

Automatic test started, see https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/pipelines/1803363/builds

gpetruc-bot commented 4 years ago

Please update PhysicsTools/NanoAOD/python/nanoDQM_cfi.py: take this patch or run prepareDQM.py -d -u nano_file_mc.root, and then if needed adjust the plot range using some human common sense.

mbluj commented 4 years ago

Hello, I checked issues found in the tests and it is what I see:

DQM: I am not able to upload auto-generated patch from here due to 404 error, but I do not see any issue with a root file from my private processing. I suppose that the issue is because auto-generation of DQM configuration is run with an era for which the new tauID is not added to NanoAOD, so the patch actually proposes to remove what is added in https://github.com/cms-nanoAOD/cmssw/pull/521/files#diff-7146a00294cb1f815c6b4b9b99641de3R570. I propose to not touch it.
Failures in 106X UL workflows: The new tauID is not present in MiniAOD with current 106X UL processing, but it is going to be present in UL reMiniAOD which is still being prepared. However, NanoAOD should skip it for the run2_nanoAOD_106Xv1 era (see here) which in my understanding corresponds with current UL MiniAOD. It does the same for older 9XY, 102X eras which run smoothly. If there is another era for NanoAOD on top of current UL MiniAOD e.g. run2_nanoAOD_106Xv2, I will add the new tauID to a tau sequence for this era so that it is available for NanoAOD - please let me know.

mariadalfonso commented 4 years ago

@mbluj For the 10_6 the UL17 runs ok, the UL16 and UL18 fails because of What's different between the years ?

----- End Fatal Exception ------------------------------------------------- 2875 ----- Begin Fatal Exception 17-Jul-2020 10:48:33 CEST----------------------- 2876 An exception of category 'Key not found' occurred while 2877 [0] Processing Event run: 320822 lumi: 4 event: 6385769 stream: 0 2878 [1] Running path 'nanoAOD_step' 2879 [2] Calling method for module SimpleCandidateFlatTableProducer/'tauTable' 2880 Exception Message: 2881 pat::Tau: the ID againstElectronDeadECAL can't be found in this pat::Tau.

mbluj commented 4 years ago

I suppose that NanoAOD configs are different rather than input samples. Could you check or let me know how to check cmsDriver commands used in the tests for UL 16, 17, 18, please? I mean non of UL 16, 17, 18 MiniAOD samples consists of the ID againstElectronDeadECAL within slimmedTaus collection.

mariadalfonso commented 4 years ago

@mbluj

you can see mc106Xul18, mc106Xul17, mc106Xul16 defined here https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L84 https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L99 https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L114

and indeed the ERA=run2_nanoAOD_106Xv1 is only set for the ul17, I try to update from my side

mbluj commented 4 years ago

@mbluj

you can see mc106Xul18, mc106Xul17, mc106Xul16 defined here https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L84 https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L99 https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L114

and indeed the ERA=run2_nanoAOD_106Xv1 is only set for the ul17, I try to update from my side

Yes, I have just checked the configurations you pointed me to, so I confirm that the run2_nanoAOD_106Xv1 is set only for the UL17, and that it is the cause of failure for UL16,18.

Now, the question is if you foresee a new version of NanoAOD (say 106Xv2) on current UL MiniAOD or the new version is going to be run with (not ready, yet) UL reMiniAOD samples. If the letter will happen nothing needs be modified in this PR. However, if former is expected a new era should be added and the anti-e DeadECal tauID should be run for this era on top of MiniAOD to enter to NanoAOD.

gpetruc-bot commented 4 years ago

Automatic test started, see https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/pipelines/1804191/builds

mariadalfonso commented 4 years ago

@mbluj you can see mc106Xul18, mc106Xul17, mc106Xul16 defined here https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L84 https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L99 https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/blob/master/scripts/run_test_long.sh#L114 and indeed the ERA=run2_nanoAOD_106Xv1 is only set for the ul17, I try to update from my side

Yes, I have just checked the configurations you pointed me to, so I confirm that the run2_nanoAOD_106Xv1 is set only for the UL17, and that it is the cause of failure for UL16,18.

Now, the question is if you foresee a new version of NanoAOD (say 106Xv2) on current UL MiniAOD or the new version is going to be run with (not ready, yet) UL reMiniAOD samples. If the letter will happen nothing needs be modified in this PR. However, if former is expected a new era should be added and the anti-e DeadECal tauID should be run for this era on top of MiniAOD to enter to NanoAOD.

I fixed those eras for 10_6, for the time being we do foresee the 106Xv2, we will need to add the 112X eras later. Let's see if this fix the issue with the testing

gpetruc-bot commented 4 years ago

Please update PhysicsTools/NanoAOD/python/nanoDQM_cfi.py: take this patch or run prepareDQM.py -d -u nano_file_mc.root, and then if needed adjust the plot range using some human common sense.

mbluj commented 4 years ago

I fixed those eras for 10_6, for the time being we do foresee the 106Xv2, we will need to add the 112X eras later. Let's see if this fix the issue with the testing

Yes, 112X eras could be need too, to avoid similar problems in a sibling PR to 112X.

Concerning 106Xv2: please let me know when it is available. This will require modification of this PR or preparation of a new one to enable running the anti-e DeadECal tauID on top of MiniAOD.

mbluj commented 4 years ago

Please update PhysicsTools/NanoAOD/python/nanoDQM_cfi.py: take this patch or run prepareDQM.py -d -u nano_file_mc.root, and then if needed adjust the plot range using some human common sense.

It is because in the era used to test DQM content the anti-e DeadECal tauId is not added to NanoAOD content. Should change in DQM be removed from this PR?

mariadalfonso commented 4 years ago

I fixed those eras for 10_6, for the time being we do foresee the 106Xv2, we will need to add the 112X eras later. Let's see if this fix the issue with the testing

Yes, 112X eras could be need too, to avoid similar problems in a sibling PR to 112X.

Concerning 106Xv2: please let me know when it is available. This will require modification of this PR or preparation of a new one to enable running the anti-e DeadECal tauID on top of MiniAOD.

sorry, I had a typo in my previous reply, I do NOT see the 106Xv2 in the plan, but I need to better discuss after it's clear the path for the UL-mini and UL-remini

mbluj commented 4 years ago

I fixed those eras for 10_6, for the time being we do foresee the 106Xv2, we will need to add the 112X eras later. Let's see if this fix the issue with the testing

Yes, 112X eras could be need too, to avoid similar problems in a sibling PR to 112X. Concerning 106Xv2: please let me know when it is available. This will require modification of this PR or preparation of a new one to enable running the anti-e DeadECal tauID on top of MiniAOD.

sorry, I had a typo in my previous reply, I do NOT see the 106Xv2 in the plan, but I need to better discuss after it's clear the path for the UL-mini and UL-remini

OK, I see. Anyway situation is as follows:

the anti-e deadECal tauID is not present in UL MiniAODs and it is switched off in 106Xv1 NanoAOD
the anti-e deadECal tauID will be present in UL reMiniAODs and will be present in NanoAOD run on top of it with any era different than currently defined ones, e.g. 106Xv1, 102Xv*, etc.,
if there is a plan to run a new version NanoAOD on top of UL MiniAODs (w/o reMiniAOD) it will be good to run the anti-e deadECal tauID as part of NanoAOD sequence to be included in NanoAOD samples. This will require extension of tau sequence for NanoAOD ,
concerning 112X eras: they are actually not needed for the anti-e deadECal tauID assuming that NanoAOD is not run on top of MiniAOD with CMSSW < 112X

mbluj commented 4 years ago

Closing as discussed in #535.

cms-nanoAOD / cmssw

Add tauID against electrons in dead ECal regions (106X) #521

PR description:

PR validation: