New MVA-based Tau-Ids (update) #146

mbluj · 2018-03-27T14:08:31Z

The MVA-based Tau-IDs are retrained with 94X Fall17 samples and should be evaluated using MiniAOD Taus (skimmedTaus) and added to NanoAOD. This PR is meant to supersede its initial version (#108) based on MVA training with 92X Summer17 samples.

The commits change three files:

RecoTauTag/Configuration/python/loadRecoTauTagMVAsFromPrepDB_cfi.py with definition of MVA payloads (new ones are added),
PhysicsTools/NanoAOD/python/taus_cff.py where new Tau-Ids are evaluated and added to the Tau table
PhysicsTools/NanoAOD/python/nanoDQM_cfi.py

Notes:

This PR bases on CMSSW_9_4_5_cand1 and master of NanoAOD repository.
Implementation in the PR is expected to give same results on MiniAOD v1 and v2 thanks to employing run2_miniAOD_94XFall17 era for v2. However, it crashes with v1 inputs due to some electron dependences
It is considered to add two int numbers to code WPs of two cut-based tau-Ids as requested by end users.

FYI, @veelken @steggema @ohlushch

veelken · 2018-03-27T14:31:03Z

Dear all,

I see that the two int numbers for the cut-based tau ID have not been added to PhysicsTools/NanoAOD/python/taus_cff.py . Can these please still be added ? I think the option to switch to the cut-based tau ID discriminators may be a useful option in case of problems with data/MC differences in the MVA tau ID input variables (the Tau POG is seeing some evidence for such data/MC differences right now and is investigating).

steggema · 2018-03-27T14:46:26Z

Hi Michal,
Thanks for the fast turnaround! We agreed yesterday to support both the 2017 MC v1-based and 2017 MC v2-based trainings. Do I read correctly that this PR adds the v2-based training, plus reverts to the 2015-based training for NanoAOD production based on MiniAOD v2? I think it would be better to offer the 2017 MC v1-based training, either in addition or replacing the 2015 training.

gpetruc

use run2_nanoAOD_94XMiniAODv1/v2 eras for 94X
it's not clear what's happening on 80X

gpetruc · 2018-03-27T14:35:02Z

PhysicsTools/NanoAOD/python/taus_cff.py

+    cut = cms.string("pt > 18 && tauID('decayModeFindingNewDMs') && (tauID('byLooseCombinedIsolationDeltaBetaCorr3Hits') || tauID('byVLooseIsolationMVArun2v1DBoldDMwLT') || tauID('byVLooseIsolationMVArun2v1DBnewDMwLT') || tauID('byVLooseIsolationMVArun2v1DBdR03oldDMwLT') || tauID('byVVLooseIsolationMVArun2v1DBoldDMwLT2017v2') || tauID('byVVLooseIsolationMVArun2v1DBnewDMwLT2017v2') || tauID('byVVLooseIsolationMVArun2v1DBdR03oldDMwLT2017v2'))")
+)
+run2_miniAOD_94XFall17.toModify(finalTaus,
+          cut =  cms.string("pt > 18 && tauID('decayModeFindingNewDMs') && (tauID('byLooseCombinedIsolationDeltaBetaCorr3Hits') || tauID('byVLooseIsolationMVArun2v1DBoldDMwLT2015') || tauID('byVLooseIsolationMVArun2v1DBnewDMwLT') || tauID('byVLooseIsolationMVArun2v1DBdR03oldDMwLT') || tauID('byVVLooseIsolationMVArun2v1DBoldDMwLT2017v2') || tauID('byVVLooseIsolationMVArun2v1DBnewDMwLT2017v2') || tauID('byVVLooseIsolationMVArun2v1DBdR03oldDMwLT2017v2'))")
 )


don't use run2_miniAOD_94XFall17 era
Have the default config to be that for 94XMiniAODv2, and use run2_nanoAOD_94XMiniAODv1 to adapt to MiniAOD v1
(I'm putting just one comment but it is true for all uses of this era in the file)

OK, thanks for letting me know, I will correct it.

gpetruc · 2018-03-27T14:40:38Z

RecoTauTag/Configuration/python/loadRecoTauTagMVAsFromPrepDB_cfi.py

@@ -41,6 +41,8 @@
 }


diff too is not being too smart here.
I would assume this only loads new items compared to what's currently in the release, can you confirm?

Yes, I confirm that it only adds new 2017v2 payloads. In particular, it means that standard RECO and MiniAOD v1 and v2 workflows are not broken.

gpetruc · 2018-03-27T14:47:43Z

PhysicsTools/NanoAOD/python/taus_cff.py

+          tauIDSources = _tauIDSourcesExt
+)
+
+patTauMVAIDsSeq += slimmedTausUpdated


can you put all this in a separate cff file (e.g. taus_updatedMVAIds_cff) ?

gpetruc · 2018-03-27T14:50:25Z

PhysicsTools/NanoAOD/python/taus_cff.py

+    + patTauDiscriminationByTightIsolationMVArun2v1DBoldDMwLT2015
+    + patTauDiscriminationByVTightIsolationMVArun2v1DBoldDMwLT2015
+    + patTauDiscriminationByVVTightIsolationMVArun2v1DBoldDMwLT2015
+)


is 2015 what is in current miniAOD v1?

Yes, exactly

mbluj · 2018-03-27T19:40:06Z

Hi

On Tue, Mar 27, 2018 at 4:31 PM, Christian Veelken ***@***.*** > wrote: Dear all, I see that the two int numbers for the cut-based tau ID have not been added to PhysicsTools/NanoAOD/python/taus_cff.py . Can these please still be added ? I think the option to switch to the cut-based tau ID discriminators may be a useful option in case of problems with data/MC differences in the MVA tau ID input variables (the Tau POG is seeing some evidence for such data/MC differences right now and is investigating).

It is in plans, i.e. I will add it as soon as I can (hopefully by tomorrow about noon). <#m_-5256863754617431948_DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

mbluj · 2018-03-27T19:43:02Z

On Tue, Mar 27, 2018 at 4:46 PM, Jan Steggemann ***@***.***> wrote: Hi Michal, Thanks for the fast turnaround! We agreed yesterday to support both the 2017 MC v1-based and 2017 MC v2-based trainings. Do I read correctly that this PR adds the v2-based training, plus reverts to the 2015-based training for NanoAOD production based on MiniAOD v2? I think it would be better to offer the 2017 MC v1-based training, either in addition or replacing the 2015 training.

OK, it can be done like this, i.e. 2017v1 can be added. The issue with the 2017v1 is that it exists only for oldDMs dR=0.5 (however it is most used version). <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> Virus-free. www.avast.com <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

mbluj · 2018-03-28T10:43:12Z

Dear All,
the two last commits address comments on:

moving definitions of MVA tau-Ids to a separate cff file
usage of correct eras (hopefully it is fine now)
adding MVAIso 2017v1 which is supported by Tau POG.

It does not address, however, the request to add cut-based tau-Ids as due to some reasons there is a problem with parsing of mathematical expressions like: "tauID('byCombinedIsolationDeltaBetaCorrRaw3Hits')<1.5" ("<" sign looks be a problem).
Advises how to cope with this problem are welcome.

gpetruc · 2018-03-28T11:24:48Z

Hi @mbluj,
Where do you need to parse the x < 1.5 ?
If it's in the table producer, then comparisons should be supported for variables of type bool, as the physicstools parser grammar allows them in cuts but not in expressions.
If you need it in an expression, e.g. to say 1*(x<1.5) + 2*(y>4.7) then you need to use the ternary operator but with one extra question mark at the beginning (because of limitations in the parser of the grammar), so the above would become (? x<1.5 ? 1 : 0) + (? y>4.7 ? 2 : 0)

mbluj · 2018-03-28T11:32:18Z

Hi,

On Wed, Mar 28, 2018 at 1:24 PM, Giovanni Petrucciani < ***@***.***> wrote: Hi @mbluj <https://github.com/mbluj>, Where do you need to parse the x < 1.5 ? If it's in the table producer, then comparisons should be supported for variables of type bool, as the physicstools parser grammar allows them in cuts but not in expressions. If you need it in an expression, e.g. to say 1*(x<1.5) + 2*(y>4.7) then you need to use the ternary operator but with one extra question mark at the beginning (because of limitations in the parser of the grammar), so the above would become (? x<1.5 ? 1 : 0) + (? y>4.7 ? 2 : 0)

Yes, it is expected to go to the table producer. I have checked that it indeed works for a single bool, but what I want is to store a few of bools (a set of WPs) in one integer (uint8), like this (a simplified example): idCombIsodR03 = Var( "1 * (tauID('chargedIsoPtSumdR03') < 2.5) + " "2 * (tauID('chargedIsoPtSumdR03') < 1.5) ", "uint8", doc="Combined isolation WP" ) I will try to adapt to your proposal, but it looks that it will produce a quite complicated expression :(

gpetruc-bot · 2018-03-28T12:40:54Z

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

gpetruc-bot

Automatic test report for 343947

gitlab pipeline at https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/pipelines/343947/builds
status: FAILURE (see link above for failed jobs)

Code integration

Code checks not run for this PR (no source files modified)

Tests

Test mc_94Xv2: passed
Test mc_94X: passed

arizzi · 2018-03-28T13:09:25Z

please use the proper Era Switches to support MC campaigns
94X MiniAOD v1
94X MiniAOD v2
80X MiniAOD
and 92X , 80X for data

mbluj · 2018-03-28T13:09:44Z

It looks that 80X jobs fail. How they should be setup? Same as MiniAODv1 ones? Should I use a specific era?
OK, it looks that it was answered in comment by @arizzi
It looks I need a clarification:

for Nano at MiniAODv1/v2 with 94X I am using run2_nanoAOD_94XMiniAODv1/v1
for Nano at MiniAOD with 92X I am going to use run2_nanoAOD_92X, correct?
while for 80X I do not see anything which looks sensible, should I use run2_miniAOD_80XLegacy? Or run2_nanoAOD_92X?

arizzi · 2018-03-28T13:55:03Z

there are specific eras for the various scenarios
https://github.com/cms-sw/cmssw/blob/master/Configuration/StandardSequences/python/Eras.py#L47-L48

mbluj · 2018-03-28T14:03:06Z

On Wed, Mar 28, 2018 at 3:55 PM, arizzi ***@***.***> wrote: there are specific eras for the various scenarios https://github.com/cms-sw/cmssw/blob/master/Configuration/ StandardSequences/python/Eras.py#L47-L48

Thank you, I know this. But while meaning and use cases for ' run2_miniAOD_94XFall17', 'run2_nanoAOD_92X' and 'run2_nanoAOD_92X' are more or less clear I am not sure what should be used for 80X as there is not specific era for nanoAOD with 80X. And it makes difference if it will be 'run2_miniAOD_80XLegacy' or something else.

mbluj · 2018-03-28T14:55:25Z

Now eras should correctly handle 94X MiniAOD v1 and v1, 92X MiniAOD, and 80X MiniAOD and LegacyMiniAOD.

Concerning cut-based isolation tau-Ids:

WPs for dR=0.5 were trivially added as they are present in MiniAOD,
while for dR=0.3 one has to use a set ternary operators to implement logic of cuts. I have tried several possible implementations (one of which is commented out in git repo), but although the implementations technically work they do not return sensible results. It could be that the expressions are too complicated for the string parser.

P.S. I'm away since ~5pm today and will be able provide only reduced fixes to the PR until end of Easter.

arizzi · 2018-04-04T06:50:01Z

are the missing variables in DQM produced in some of the other eras? @gpetruc which era is the dqm tested on ?

gpetruc-bot · 2018-04-04T07:57:32Z

the DQM config is tested on 94XMiniAODv2 (missing variables are ignored) I haven't tried to make the dqm auto-update script to work with eras. Giovanni

…

On Wed, Apr 4, 2018 at 8:50 AM, arizzi ***@***.***> wrote: are the missing variables in DQM produced in some of the other eras? @gpetruc <https://github.com/gpetruc> which era is the dqm tested on ? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#146 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AfBP0d9r5Al7Pv2dSdKCGcfDUYF71oSHks5tlG0agaJpZM4S8_nl> .

mbluj · 2018-04-04T09:52:24Z

On Wed, Apr 4, 2018 at 8:50 AM, arizzi ***@***.***> wrote: are the missing variables in DQM produced in some of the other eras? @gpetruc <https://github.com/gpetruc> which era is the dqm tested on ?

Content of the tau table (other tables as well?) depends on era and it could be indeed useful to make DQM also era depended. Anyway, I have defined plots which make sense for all areas, i.e. there are histograms which correspond with variables present in some eras but not in others which triggers the warning by the DQM test.

arizzi · 2018-04-04T12:14:34Z

@mbluj giovanni suggested that you store in the cfi only the 94X latest (i.e. default) config and use the DQM_cff.py to apply era switches that could add back the legacy variables.
Can you try implementing it? (let me know if you can do it today, otherwise I may try to have a look at it at some point)

mbluj · 2018-04-04T12:29:50Z

On Wed, Apr 4, 2018 at 2:14 PM, arizzi ***@***.***> wrote: @mbluj <https://github.com/mbluj> giovanni suggested that you store in the cfi only the 94X latest (i.e. default) config and use the DQM_cff.py to apply era switches that could add back the legacy variables. Can you try implementing it? (let me know if you can do it today, otherwise I may try to have a look at it at some point)

OK, I can spend some time on it. I'll let you know if I fail.

mbluj · 2018-04-04T13:34:55Z

Era dependence added to NanoDQM_cff for tau plots. However, I have not idea how to test it properly...

gpetruc-bot · 2018-04-04T13:36:06Z

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

* Add payloads for tau-Id MVAIso 2017v2 * Add MVAIso tau-Id 2017v2 * Move definitions of new MVA tau-Ids to a specific cff file * Use correct eras, add MVAIso2017v1 * Add cut-based isolation WPs * correct eras, comment out idCombIsodR03 due to incorrect behavior * further era corrections * fix bug in eras handling * add rawIso dR=0.3, remove cut-base WPs and footprintCorr * remove MVAIso oldDM dR=0.3 and newDM with 2015 training * remove instead commenting out * keep all MVAIso 2015 Tau-Ids and remove 2017v1/2 ones for 80X * Add era modifiers to NanoDQM

gpetruc-bot

Automatic test report for 348530

gitlab pipeline at https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/pipelines/348530/builds
outputs at https://test-cms-nanoaod-integration.web.cern.ch/integration/test_pr_146/

Code integration

Code checks not run for this PR (no source files modified)

Tests

Long test data80X (10000 events): passed, with differences; dqm plots: all, diff
Long test data80Xhip (3000 events): passed, with differences; dqm plots: all, diff
Long test data94X (10000 events): passed, no significant changes; dqm plots: all, diff
Long test data94Xv2 (10000 events): passed, no significant changes; dqm plots: all, diff
Long test mc80X (10000 events): passed, with differences; dqm plots: all, diff
Long test mc94X (10000 events): passed, no significant changes; dqm plots: all, diff
Long test mc94Xv2 (9000 events): passed, with differences; dqm plots: all, diff
Test mc_94Xv2: passed
Test mc_94X: passed
Test mc_80X: passed
Test data_94Xv2: passed
Test data_94X: passed
Test data_80X: passed

Disk size report

Sample	kb/event	ref kb/event	diff
TTbar MC 94Xv1	1.544	1.532	0.012 ( +0.8% )
TTbar MC 94Xv2	1.563	1.549	0.014 ( +0.9% )
TTbar MC 80X	1.569	1.569	-0.000 ( -0.0% )
Data 94Xv1	0.620	0.613	0.008 ( +1.3% )
Data 80X	0.558	0.557	0.001 ( +0.1% )
Data 80X, Mu Run2016E	0.558	0.557	0.001 ( +0.1% )

arizzi · 2018-04-04T14:28:21Z

cherry picked on master-cmsswmaster and masterMergedAndNoData-integration

arizzi · 2018-04-04T14:29:56Z

cms-sw#22840

vlimant · 2024-01-26T14:05:00Z

@ALL what was the intention is pulling taggers from outside of the GT here ?

mbluj · 2024-01-26T14:24:45Z

Hello,
This was "temporary" solution to enable access to tauID payloads before they were available via GT (used around beginning of Run-2). Then it was used for testing purposes, etc. Anyway, up to my knowledge this was removed from CMSSW already some time ago (about ~2017?), but let me check. Moreover, tauIDs using these payloads are generally not supported for some time already.

mbluj · 2024-01-26T14:49:18Z

After further checks: there is still possibility to run old tauIDs (early Run-2) accessing their payloads w/o GT for certain Run-2 nanoAOD eras. This is steered by the following cff: PhysicsTools/NanoAOD/python/taus_updatedMVAIds_cff.py. The idea was to enable access to consistent set of versions of those IDs independently on version of miniAOD which was used to produce given nanoAOD. It is however not used for long time at least since Run2 legacy production (10_2?). I do not remember why it was kept for newer releases, probably was overlooked. It is not needed to read Run-2 UL samples (10_6) or even to reproduce most recent versions of the mentioned tauID on top of the UL samples as their payloads are accessible via GT.
I think I can prepare quickly a PR removing this stuff from NanoAOD sequences, i.e. remove PhysicsTools/NanoAOD/python/taus_updatedMVAIds_cff.py.

vlimant · 2024-01-26T14:54:34Z

thanks for checking, please prepare a PR indeed.
N.B. we have workflows running from 10.6 MINI, modified with the era run2_nanoAOD_106Xv2 that seem to still pull in the ID from outside the GT ; for these workflows, which we still want to maintain, the corresponding GT will have to be updated accordingly

mbluj · 2024-01-26T16:35:07Z

thanks for checking, please prepare a PR indeed. N.B. we have workflows running from 10.6 MINI, modified with the era run2_nanoAOD_106Xv2 that seem to still pull in the ID from outside the GT ; for these workflows, which we still want to maintain, the corresponding GT will have to be updated accordingly

I agree, it makes sense to maintain run2_nanoAOD_106Xv2 era as it is the only way to produce Nano with UL samples with new releases. But, it is strange to me that old tauID is used by an official workflow with this era. I have to check this in details. Is thre another wf. (in matrix tests) affected by this issue than one reported by Kati?

mbluj added 3 commits March 26, 2018 16:56

Merged master from repository cms-nanoAOD with cms-merge-topic

d401c19

Add payloads for tau-Id MVAIso 2017v2

33919f8

Add MVAIso tau-Id 2017v2

7bc2e4a

gpetruc requested changes Mar 27, 2018

View reviewed changes

mbluj added 2 commits March 27, 2018 22:08

Move definitions of new MVA tau-Ids to a specific cff file

505a673

Use correct eras, add MVAIso2017v1

7500553

Add cut-based isolation WPs

761163c

arizzi added the to be tested by bot label Mar 28, 2018

gpetruc-bot added test scheduled (bot) and removed to be tested by bot test scheduled (bot) labels Mar 28, 2018

gpetruc-bot added the test started (bot) label Mar 28, 2018

gpetruc-bot previously requested changes Mar 28, 2018

View reviewed changes

gpetruc-bot added test failed (bot) and removed test started (bot) labels Mar 28, 2018

mbluj added 2 commits March 28, 2018 16:29

correct eras, comment out idCombIsodR03 due to incorrect behavior

c87de85

further era corrections

4b6743d

Add era modifiers to NanoDQM

60b48d2

arizzi added to be tested by bot and removed dqm config update (bot) test ok (bot) labels Apr 4, 2018

gpetruc-bot added test scheduled (bot) and removed to be tested by bot test scheduled (bot) labels Apr 4, 2018

gpetruc-bot added the test started (bot) label Apr 4, 2018

arizzi merged commit 0733e5e into cms-nanoAOD:master Apr 4, 2018

gpetruc-bot reviewed Apr 4, 2018

View reviewed changes

gpetruc-bot added test ok (bot) and removed test started (bot) labels Apr 4, 2018

mbluj deleted the CMSSW_9_4_X_MVAIso2017v2ForNano branch September 7, 2018 14:40

vlimant mentioned this pull request Jan 26, 2024

NANO (and MINI) pulling conditions from outside GT cms-sw/cmssw#43797

Closed

New MVA-based Tau-Ids (update) #146

New MVA-based Tau-Ids (update) #146

Conversation

mbluj commented Mar 27, 2018

veelken commented Mar 27, 2018

steggema commented Mar 27, 2018

gpetruc left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbluj commented Mar 27, 2018 via email

mbluj commented Mar 27, 2018 via email

mbluj commented Mar 28, 2018

gpetruc commented Mar 28, 2018

mbluj commented Mar 28, 2018 via email

gpetruc-bot commented Mar 28, 2018

gpetruc-bot left a comment

Choose a reason for hiding this comment

Automatic test report for 343947

Code integration

Tests

arizzi commented Mar 28, 2018

mbluj commented Mar 28, 2018 • edited Loading

arizzi commented Mar 28, 2018

mbluj commented Mar 28, 2018 via email

mbluj commented Mar 28, 2018 • edited Loading

arizzi commented Apr 4, 2018

gpetruc-bot commented Apr 4, 2018 via email

mbluj commented Apr 4, 2018 via email

arizzi commented Apr 4, 2018

mbluj commented Apr 4, 2018 via email

mbluj commented Apr 4, 2018

gpetruc-bot commented Apr 4, 2018

gpetruc-bot left a comment

Choose a reason for hiding this comment

Automatic test report for 348530

Code integration

Tests

Disk size report

arizzi commented Apr 4, 2018

arizzi commented Apr 4, 2018

vlimant commented Jan 26, 2024

mbluj commented Jan 26, 2024

mbluj commented Jan 26, 2024

vlimant commented Jan 26, 2024

mbluj commented Jan 26, 2024

mbluj commented Mar 28, 2018 •

edited

Loading

mbluj commented Mar 28, 2018 •

edited

Loading