Developments to provide facilitated access to new TauID information (all high level changes) #18244

roger-wolf · 2017-04-06T11:35:46Z

This is a PR that had been stalled on 90X since cmssw was just in the transition to 91X. Comments from RECO formerly made here (#17624) have now been addressed and the PR migrated to 91X. I'm copying the comments of the former PR here once again to make clear hat this is about. Note that these developments are most relevant for current analyses so upon integration they will require backports down to 81X.

Dear colleagues,

this pull request comprises three developments that have been made to facilitate the access to new TauID information that is now available based on trainings made on the whole 2016 dataset.

configuring access to the new MVATauID training that has been made on the full 2016 dataset.
introducing a bypass for a variable that could be missing on miniAOD (phiAtEcalEntrace) -- when set to true by configuration this variable can be approximated from existing miniAOD information. This is in the first place in preparation for a backport to 80X, where this issue occurs. By default and for CMSSW>=8_1_X the parameter will be false.
adding a producer plugin (PATTauIDEmbedder) that allows to write a slimmedTau collection back into the EDM event content with updated MVAIsolation and MVA electron identification.

All these changes are "high level" with effect only on the analysis level on already existing miniAOD. They are meant for the currently ongoing analyses with the actual target to be backported down to CMSSW_8_0_X.

Neither unit/matrixTests nor DQM plots tracking physics performance should be affected by these changes.

Cheers,
Roger

…t (Alex)

…o ECAL entrace in case that phiAtEcal is not part of miniAOD

…that could also be made persistent

…auMVAs

…au-pog_miniAOD-tauMVAs

…ervice

cmsbuild · 2017-04-06T11:36:08Z

A new Pull Request was created by @roger-wolf (Roger Wolf) for master.

It involves the following packages:

DataFormats/PatCandidates
RecoTauTag/Configuration
RecoTauTag/RecoTau

@perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please review it and eventually sign? Thanks.
@gpetruc, @gouskos, @cbernet this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here

slava77 · 2017-04-06T12:33:22Z

@cmsbuild please test

cmsbuild · 2017-04-06T12:33:42Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18977/console Started: 2017/04/06 14:43

cmsbuild · 2017-04-06T14:02:49Z

+1
Tested at: 7a2c1c4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18244/18977/summary.html

cmsbuild · 2017-04-06T14:02:52Z

Comparison job queued.

cmsbuild · 2017-04-06T16:22:22Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18244/18977/summary.html

Comparison Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
results differ 1232 all_OldVSNew_TTbarPUwf50202p0 JR results differ 376 all_mini_OldVSNew_TTbarPUwf50202p0 SUMMARY Reco comparison results: 1608 differences found in the comparisons
DQMHistoTests: Total files compared: 23
DQMHistoTests: Total histograms compared: 1913135
DQMHistoTests: Total failures: 36094
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1876868
DQMHistoTests: Total skipped: 173
DQMHistoTests: Total Missing objects: 0
Checked 94 log files, 14 edm output root files, 23 DQM output files

…, requires a few knock-on adaptations in several files

cmsbuild · 2017-04-24T23:39:56Z

Pull request #18244 was updated. @perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please check and sign again.

slava77 · 2017-04-24T23:48:28Z

RecoTauTag/RecoTau/plugins/PATTauDiscriminationAgainstElectronMVA6.cc

@@ -104,7 +112,7 @@ double PATTauDiscriminationAgainstElectronMVA6::discriminate(const TauRef& theTa
 	double deltaREleTau = deltaR(theElectron.p4(), theTauRef->p4());
 	deltaRDummy = deltaREleTau;
 	if( deltaREleTau < 0.3 ){ 	
-	  double mva_match = mva_->MVAValue(*theTauRef, theElectron, usePhiAtEcalEntranceExtrapolation_);	  
+	  double mva_match = mva_->MVAValue(*theTauRef, theElectron, usePhiAtEcalEntranceExtrapolation_, bField_);	  


why not set bField in the mva::beginEvent call ?
It seems like a simpler solution with fewer changes in interfaces.

slava77 · 2017-04-24T23:49:20Z

RecoTauTag/RecoTau/src/AntiElectronIDMVA6.cc

 {
  bool result = false;
-  double bField = 3.8; // hopefully Teslas are proper units (better to take it from event setup) 
+  //double bField = 3.8; // hopefully Teslas are proper units (better to take it from event setup) 


the comment can be now removed

…lectronIDMVA6

cmsbuild · 2017-04-25T08:09:15Z

Pull request #18244 was updated. @perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please check and sign again.

slava77 · 2017-04-25T12:20:50Z

@cmsbuild please test

cmsbuild · 2017-04-25T12:21:06Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19372/console Started: 2017/04/25 14:22

cmsbuild · 2017-04-25T14:22:16Z

+1
Tested at: 48fa4c5
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18244/19372/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
b6bfaa5
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18244/19372/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18244/19372/git-merge-result

cmsbuild · 2017-04-25T14:22:20Z

Comparison job queued.

cmsbuild · 2017-04-25T16:39:22Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18244/19372/summary.html

Comparison Summary:

You potentially added 29 lines to the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 23
DQMHistoTests: Total histograms compared: 1780076
DQMHistoTests: Total failures: 14948
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1764955
DQMHistoTests: Total skipped: 173
DQMHistoTests: Total Missing objects: 0
Checked 94 log files, 14 edm output root files, 23 DQM output files

slava77 · 2017-04-25T17:25:13Z

+1

for #18244 48fa4c5

changes are in line with the description: default behavior of algorithms running in reco/miniAOD is unchanged
jenkins tests pass and comparisons with baseline show no differences

davidlange6 · 2017-04-26T12:17:57Z

RecoTauTag/RecoTau/src/AntiElectronIDMVA6.cc

+#include "MagneticField/Engine/interface/MagneticField.h"
+#include "MagneticField/Records/interface/IdealMagneticFieldRecord.h"
+
+#include <TMath.h>


hi @roger-wolf - is this needed?

Hi David,

I'm not sure I understand what you are referring to, therefore I'll answer to both interpretations that I'm having:

(1)
The MagneticFiled headers are needed for the track extrapolation we are doing in case that the variable "phiAtEcalEntrace" has not been pre-calculated to be saved in miniAOD (the Bfield is used in function atECalEntrance).

(2)
TMath actually is not used any more as it seem to me. If you want me to do it I could delete the line and recommit, while I'm a bit hesitant to start the whole signing and testing procedure again on this long thread.

Cheers,
Roger

it was about the TMath.h include - please remove in a follow up PR.

davidlange6 · 2017-04-26T12:29:05Z

RecoTauTag/RecoTau/src/AntiElectronIDMVA6.cc

+      reco::Candidate const* signalCand = signalCands[o].get();
+      float phi = theTau.phi();
+      math::XYZPoint aPos;
+      if ( atECalEntrance(signalCand, aPos) == true ) phi = aPos.Phi();


Hi @roger-wolf - just to be sure I follow - this phi variable ends up meaning "phi at ecal" if the particle intersects the ecal, otherwise it is phi at the primary vertex?

Hi David,

yes this is the way also the training has been done.

Cheers,
Roger

roger-wolf · 2017-04-28T09:04:28Z

Hi all,

I think the last missing signatures are not more than formalities. @davidlange6 : would you like me to remove the header in discussion above and to recommit? Otherwise just let me know if you want/need some action from my side. Could AT also sign off in case of no further issues?

Since this is a long pending PR I'd be happy to get it off the table asap.

Cheers,
Roger

roger-wolf · 2017-04-29T23:22:47Z

@monttj : Hi Tae Jeong, can you sign this thing off for us? I think this is the last step for us to get this PR off the table. Thanx a lot!

Cheers,
Roger

roger-wolf added 12 commits February 24, 2017 12:50

Merge branch 'from-CMSSW_9_0_X_2017-02-22-2300' into CMSSW_9_0_X_tau-pog

dd670c7

Access to new MVA tauID trainings on full 2016 dataset + internal tes…

229164f

…t (Alex)

workaround by Anne-Catherine that allows manual track extrapolation t…

f9ea9d1

…o ECAL entrace in case that phiAtEcal is not part of miniAOD

enable re-embedding of new tauID info into an updated patTau object, …

ca50fe2

…that could also be made persistent

adding missing config to Reco cfi

fb09f6f

adding missing file

0432ec5

Merge branch 'from-CMSSW_9_0_X_2017-02-26-2300' into CMSSW_9_0_X_tau-pog

f4d6d29

Merge branch 'CMSSW_9_0_X_tau-pog' into CMSSW_9_0_X_tau-pog_miniAOD-t…

d550ac3

…auMVAs

Merge branch 'CMSSW_9_0_X_tau-pog_miniAOD-tauMVAs' into CMSSW_9_1_X_t…

54a1132

…au-pog_miniAOD-tauMVAs

repairing the validation of minAOD branching by correct use of TFileS…

8917638

…ervice

addressing requests and comment by RECO from last time this PR was made

247debd

removing whitespace

7a2c1c4

cmsbuild added this to the Next CMSSW_9_1_X milestone Apr 6, 2017

cmsbuild added analysis-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 6, 2017

cmsbuild added tests-started and removed tests-pending labels Apr 6, 2017

cmsbuild added tests-approved and removed tests-started labels Apr 6, 2017

cmsbuild removed the comparison-pending label Apr 6, 2017

logic fixes - pickup Bfield from EventSetup instead of hard-coding...…

923271e

…, requires a few knock-on adaptations in several files

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels Apr 24, 2017

slava77 reviewed Apr 24, 2017

View reviewed changes

moving readout of EventRecord for B-field from plugins to class AntiE…

48fa4c5

…lectronIDMVA6

cmsbuild added tests-started and removed tests-pending labels Apr 25, 2017

cmsbuild added tests-approved and removed tests-started labels Apr 25, 2017

cmsbuild added comparison-available and removed comparison-pending labels Apr 25, 2017

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Apr 25, 2017

davidlange6 reviewed Apr 26, 2017

View reviewed changes

davidlange6 merged commit dadb664 into cms-sw:master May 2, 2017

roger-wolf deleted the CMSSW_9_1_X_tau-pog_miniAOD-tauMVAs branch November 17, 2017 10:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Developments to provide facilitated access to new TauID information (all high level changes) #18244

Developments to provide facilitated access to new TauID information (all high level changes) #18244

roger-wolf commented Apr 6, 2017

cmsbuild commented Apr 6, 2017 •

edited by smuzaffar

slava77 commented Apr 6, 2017

cmsbuild commented Apr 6, 2017 •

edited

cmsbuild commented Apr 6, 2017

cmsbuild commented Apr 6, 2017

cmsbuild commented Apr 6, 2017

cmsbuild commented Apr 24, 2017

slava77 Apr 24, 2017

slava77 Apr 24, 2017

cmsbuild commented Apr 25, 2017

slava77 commented Apr 25, 2017

cmsbuild commented Apr 25, 2017 •

edited

cmsbuild commented Apr 25, 2017

cmsbuild commented Apr 25, 2017

cmsbuild commented Apr 25, 2017

slava77 commented Apr 25, 2017

davidlange6 Apr 26, 2017

roger-wolf Apr 26, 2017

davidlange6 May 2, 2017

davidlange6 Apr 26, 2017

roger-wolf Apr 26, 2017

roger-wolf commented Apr 28, 2017

roger-wolf commented Apr 29, 2017

Developments to provide facilitated access to new TauID information (all high level changes) #18244

Developments to provide facilitated access to new TauID information (all high level changes) #18244

Conversation

roger-wolf commented Apr 6, 2017

cmsbuild commented Apr 6, 2017 • edited by smuzaffar

slava77 commented Apr 6, 2017

cmsbuild commented Apr 6, 2017 • edited

cmsbuild commented Apr 6, 2017

cmsbuild commented Apr 6, 2017

cmsbuild commented Apr 6, 2017

cmsbuild commented Apr 24, 2017

slava77 Apr 24, 2017

Choose a reason for hiding this comment

slava77 Apr 24, 2017

Choose a reason for hiding this comment

cmsbuild commented Apr 25, 2017

slava77 commented Apr 25, 2017

cmsbuild commented Apr 25, 2017 • edited

cmsbuild commented Apr 25, 2017

cmsbuild commented Apr 25, 2017

cmsbuild commented Apr 25, 2017

slava77 commented Apr 25, 2017

davidlange6 Apr 26, 2017

Choose a reason for hiding this comment

roger-wolf Apr 26, 2017

Choose a reason for hiding this comment

davidlange6 May 2, 2017

Choose a reason for hiding this comment

davidlange6 Apr 26, 2017

Choose a reason for hiding this comment

roger-wolf Apr 26, 2017

Choose a reason for hiding this comment

roger-wolf commented Apr 28, 2017

roger-wolf commented Apr 29, 2017

cmsbuild commented Apr 6, 2017 •

edited by smuzaffar

cmsbuild commented Apr 6, 2017 •

edited

cmsbuild commented Apr 25, 2017 •

edited