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

Restructured code of againstElectronDeadECAL tauID (11_1_X) #30009

Merged

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented May 28, 2020

PR description:

This is backport of #29627 - actually exactly same development branch is used as in the original PR (already merged to master). Need of the backport is caused by the fact that target release series has been changed during review of the original PR.

Restructured code of againstElectronDeadECAL discriminator to run on either AOD or miniAOD inputs:

  • add a version of the discriminant to run on top of pat::Taus to be used with miniAOD data samples;
  • use eta and phi of impact position of the leading PFChargedHadron on ECal surface instead of eta and phi of tau at the vertex (taking bending of track in the magnetic field and the zVertex into account); it can be disabled for backwards compatibility;
  • restructure the code in the way that AOD and miniAOD versions of the discriminant are just different instances of the same template code;
  • move code preforming extrapolation to the ECal surface from the againstElectronMVA algorithm to an auxiliary helper class which avoids code duplication;
  • add output of the againstElectronDeadECAL discriminant produced at AOD (already present in RECO/AOD workflow) to slimmedTaus in MiniAOD

Expected some differences:

  • small difference in output of PFRecoTauDiscriminationAgainstElectronDeadECAL (AOD) discriminant because of enabled extrapolation to the ECAL (version with the extrapolation disabled gives identical results as one before the changes in this PR);
  • new tauID embedded into slimmedTaus in miniAOD.

PR validation:

Tested several times during review of original PR basing on the same development branch.

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

It is backport of #29627 (actually it uses exactly same development branch as the original/mother PR)

Christian Veelken and others added 10 commits May 7, 2020 16:08
@cmsbuild
Copy link
Contributor

cmsbuild commented May 28, 2020

A new Pull Request was created by @mbluj for CMSSW_11_1_X.

It involves the following packages:

PhysicsTools/PatAlgos
RecoTauTag/Configuration
RecoTauTag/RecoTau

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @hatakeyamak, @emilbols, @peruzzim, @seemasharmafnal, @mmarionncern, @ahinzmann, @smoortga, @jdolen, @ferencek, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @andrzejnovak, @clelange, @riga, @JyothsnaKomaragiri, @mverzett, @gpetruc, @mariadalfonso this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

backport of #29627

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 28, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6643/console Started: 2020/05/28 13:10

@cmsbuild
Copy link
Contributor

+1
Tested at: 84762b7
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ce2c7f/6643/summary.html
CMSSW: CMSSW_11_1_X_2020-05-27-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@slava77
Copy link
Contributor

slava77 commented May 28, 2020

@mbluj
IIUC, this is not required for the Phase-2 HLT TDR rereco (the main target for 11_1_X).
Is this correct?
Please clarify.
Thank you.

@mbluj
Copy link
Contributor Author

mbluj commented May 28, 2020

@mbluj
IIUC, this is not required for the Phase-2 HLT TDR rereco (the main target for 11_1_X).
Is this correct?
Please clarify.
Thank you.

No, is not required.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2460 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2780792
  • DQMHistoTests: Total failures: 48
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2780694
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
  • Checked 152 log files, 16 edm output root files, 36 DQM output files

@slava77
Copy link
Contributor

slava77 commented May 29, 2020

+1

for #30009 84762b7

  • this is identical to the master version Restructured code of againstElectronDeadECAL tauID #29627 84762b7
  • this backport is not aiming for the phase-2 HLT needs, but it will be important for potential Run-3 studies in this release cycle. The PR adds new ID variables and should not affect validation of other produced objects.

@santocch
Copy link

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_11_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_2_X is complete. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit c10dfb6 into cms-sw:CMSSW_11_1_X May 29, 2020
@mbluj mbluj deleted the CMSSW_11_1_X_tau-pog_deadECAL branch August 4, 2020 08:55
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.

6 participants