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

Tau rereco for MiniAOD of 80X legacy samples #20412

Merged
merged 9 commits into from Sep 24, 2017

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented Sep 7, 2017

This pull requests adds tau re-reconstruction to MiniAOD step for legacy re-reco of 2016 data and MC produced with CMSSW_80X. The re-reconstruction is added to MiniAOD setup (PhysicsTools/PatAlgos/python/slimming/miniAOD_tools.py) when the run2_miniAOD_80XLegacy Era is requested.

Expected effect of the PR was discussed in the xPOG meeting on 5 September 2017 [1].

Note 1: During the xPOG meeting it was agreed that the PR (when sent to official CMSSW repository) will be scrutinized, but its integration will be hold until Tau POG conveners (@steggema, @isobelojalvo) give their final green light. The procedure is to speedup the integration process.

Note 2: The PR consists of purely technical modification of PhysicsTools/PatAlgos/python/producersLayer1/tauProducer_cff.py file: it is a cleaning which avoids re-reco of cut-based tau discriminants which is not necessary. In addition, input files for tau reco tests are updated in RecoTauTag/Configuration/test/ZTT-validation.txt file.

[1] https://indico.cern.ch/event/663315/contributions/2708403/attachments/1518558/2371260/mbluj_TauId_80XLegacyMiniAOD_5Sep2017.pdf

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2017

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2017

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

It involves the following packages:

PhysicsTools/PatAlgos
RecoTauTag/Configuration

@perrotta, @cmsbuild, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @gpetruc, @gkasieczka, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @seemasharmafnal, @JyothsnaKomaragiri this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2017

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20412/547

@perrotta
Copy link
Contributor

perrotta commented Sep 7, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22773/console Started: 2017/09/07 12:31

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2017

Comparison job queued.

process.load("RecoTauTag.Configuration.RecoPFTauTag_cff")
from PhysicsTools.PatAlgos.tools.helpers import listModules
for module in listModules(process.PFTau):
_makePatTausTaskWithTauReReco.add(module)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please define tasks and derive relevant sequences from them in RecoPFTauTag_cff so that here you can import the task and simply attach it instead of constructing it in place.
We should eventually migrate to that.

@@ -20,7 +20,7 @@

makePatTausTask = cms.Task(
# reco pre-production
patHPSPFTauDiscriminationTask,
#patHPSPFTauDiscriminationTask,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this commented out code needed?
Please remove or add comments inline in the code why the commented out block is relevant

Same for the older patTauJetCorrections below

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 7, 2017

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 18 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2642440
  • DQMHistoTests: Total failures: 198
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2642053
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 14 edm output root files, 26 DQM output files

@cmsbuild
Copy link
Contributor

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

There are some workflows for which there are errors in the baseline:
1330.0 step 4
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 18 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2563849
  • DQMHistoTests: Total failures: 206
  • DQMHistoTests: Total nulls: 3
  • DQMHistoTests: Total successes: 2563454
  • DQMHistoTests: Total skipped: 186
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 14 edm output root files, 26 DQM output files

@mbluj
Copy link
Contributor Author

mbluj commented Sep 18, 2017

@slava77, Is the PR technically OK?
If so, is a decision of Tau POG conveners (@steggema, @isobelojalvo) the last missing item to merge or close the PR?
BTW, even if the decision is negative, i.e. 80X legacy MiniAOD will be performed with 2016 setup of tau-reco&id I vote to keep migration from classic sequences to tasks performed in this PR.

@slava77
Copy link
Contributor

slava77 commented Sep 18, 2017 via email

@mbluj
Copy link
Contributor Author

mbluj commented Sep 18, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Sep 18, 2017 via email

@steggema
Copy link
Contributor

I confirm for the tau POG that we want this integrated, so please proceed with the checks and the integration. Thanks!

@slava77
Copy link
Contributor

slava77 commented Sep 22, 2017

@cmsbuild please test

to refresh the tests that were deleted already

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 22, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/23180/console Started: 2017/09/22 19:09

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 18 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2691683
  • DQMHistoTests: Total failures: 227
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2691266
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 15 edm output root files, 26 DQM output files

@slava77
Copy link
Contributor

slava77 commented Sep 23, 2017

From 1325.5 (ttbar from 2016 AOD tranche IV) running on 1K events:

  • changes are only in slimmedTaus, about 7% more taus reconstructed
    all_sign965vsorig_ttbar13reminiaodwf1325p5c_pattaus_slimmedtaus__pat_obj_et

  • the addition is somewhat localized at low hcal energy and also lower pt of lead charged cand
    all_sign965vsorig_ttbar13reminiaodwf1325p5c_pattaus_slimmedtaus__pat_obj_hcalenergy
    all_sign965vsorig_ttbar13reminiaodwf1325p5c_pattaus_slimmedtaus__pat_obj_ptleadchargedcand

  • On resource use:

    • there are 73 more modules running, all tau reco related; the CPU in the ttbar sample is about 0.1s/event or 13% of the total processing time
    • the output miniAOD file changes only in the slimmedTaus, increasing the branch size by about 5% or 75 bytes/event.

This looks as expected.
However, I see no differences in the miniAOD DQM outputs, which suggests that the taus are not monitored. This should better be corrected soon to get proper validation with the standard tools.

@slava77
Copy link
Contributor

slava77 commented Sep 23, 2017

+1

for #20412 924ffa5

  • migrated tau reco to use tasks; added tau reco to the run2_miniAOD_80XLegacy era in order to use the latest tau reco algorithms for the legacy re-miniAOD using 80X inputs.
  • jenkins tests pass and comparisons with baseline show differences only in the re-miniAOD workflow in slimmedTau variables
  • more notes from local tests are available in Tau rereco for MiniAOD of 80X legacy samples #20412 (comment)

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 2492027 into cms-sw:master Sep 24, 2017
@mbluj mbluj deleted the CMSSW_9_3_X_cms-tau-pog_80XLegacy branch October 10, 2023 10:05
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.

None yet

7 participants