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

DeepTau v2p5 in nanoAOD #38726

Merged

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented Jul 13, 2022

PR description:

This PR adds working point definitions for a newly integrated DeepTau v2p5.
A corresponding study with details about WP threshold derivation and tau efficiency/mis-ID rate plots for both Run2 UL and Run 3 samples can be found here.

In particular, the changes in this PR are:

  • WP thresholds values for DeepTau v2p5 are added to runTauIdMVA.py and thus to miniAOD;
  • For nanoAOD, add a dedicated set of variables _deepTauVars2018v2p5 to taus_cff.py,
  • Unify WP masking interface to a single function _tauIdWPMask();
  • Option to compute WP flags from raw scores (from_raw argument in _tauIdWPMask()) given the threshold values, instead of reading them directly from MINIAOD;
  • Change the format of storing WPs from bitmask to integer values for user-friendliness [see note 1].

Backport to 12_4_X foreseen.

[Note 1]: Change of the format of storing WPs affects also WPs of already present tauIDs and thus is not backward compatible. For instance if old versions of nano (with old eras) were produced with this PR integrated they would have format which is not backward compatible with the same versions without this PR. We think, however, that there is not realistic use-case, i.e. old versions of nano are not produced with new releases. Keeping backward compatibility is possible, but implementation will be painful. Comments from experts are welcome.

PR validation:

Successfully tested with the "limited" set of matrix tests and a custom nanoAOD production.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38726/31027

  • This PR adds an extra 104KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • PhysicsTools/NanoAOD (xpog)
  • RecoTauTag/RecoTau (reconstruction)

@gouskos, @clacaputo, @cmsbuild, @fgolf, @jpata, @mariadalfonso can you please review it and eventually sign? Thanks.
@mbluj, @gpetruc, @azotz, @swertz this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38726/31029

  • This PR adds an extra 108KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

Pull request #38726 was updated. @gouskos, @clacaputo, @cmsbuild, @fgolf, @jpata, @mariadalfonso can you please check and sign again.

@gouskos
Copy link
Contributor

gouskos commented Jul 13, 2022

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f206e7/26211/summary.html
COMMIT: d09f58d
CMSSW: CMSSW_12_5_X_2022-07-13-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38726/26211/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test runtestPhysicsToolsNanoAOD had ERRORS

RelVals-INPUT

The relvals timed out after 4 hours.

  • 1325.61325.6_TTbar_13_94Xv1NanoAODINPUT+TTbar_13_94Xv1NanoAODINPUT+NANOAODMC2017_94XMiniAODv1/step2_TTbar_13_94Xv1NanoAODINPUT+TTbar_13_94Xv1NanoAODINPUT+NANOAODMC2017_94XMiniAODv1.log
  • 1325.81325.8_TTbar_13_94Xv1NanoAODINPUT+TTbar_13_94Xv1NanoAODINPUT+NANOEDMMC2017_94XMiniAODv1+HARVESTNANOAODMC2017_94XMiniAODv1/step2_TTbar_13_94Xv1NanoAODINPUT+TTbar_13_94Xv1NanoAODINPUT+NANOEDMMC2017_94XMiniAODv1+HARVESTNANOAODMC2017_94XMiniAODv1.log

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 197 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3653966
  • DQMHistoTests: Total failures: 235
  • DQMHistoTests: Total nulls: 76
  • DQMHistoTests: Total successes: 3653633
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -30.842000000000002 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): -3.398 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 1325.81 ): -7.797 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 136.8523 ): -2.661 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@mbluj
Copy link
Contributor Author

mbluj commented Jul 14, 2022

Investigating problem with the failed unit tests.

@mbluj
Copy link
Contributor Author

mbluj commented Jul 14, 2022

Unit tests should be fixed now. Also 1325.6 and 1325.8 run at lxplus are successfully passed.

@mbluj
Copy link
Contributor Author

mbluj commented Aug 2, 2022

In d0122b1 suffixes are added to tauID modules created for MiniAOD and for NanoAOD. This avoids that modules from one workflow are overriding ones from another workflow when both workflows are run together as in 136.72412.
This commit does not solve possible issue that deepTauId is run twice, however I assume that it will not happen in a regular production, e.g. Run-2 UL miniAOD will not be produced together with NanoAOD v10 using CMSSW_12_4_X (or newer).

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38726/31381

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2022

Pull request #38726 was updated. @gouskos, @swertz, @vlimant, @clacaputo, @cmsbuild, @jpata, @mariadalfonso can you please check and sign again.

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f206e7/26594/summary.html
COMMIT: d0122b1
CMSSW: CMSSW_12_5_X_2022-08-02-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38726/26594/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 221 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3677948
  • DQMHistoTests: Total failures: 213
  • DQMHistoTests: Total nulls: 71
  • DQMHistoTests: Total successes: 3677642
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -30.846000000000004 KiB( 50 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): -3.398 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 1325.81 ): -7.797 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 136.8523 ): -2.661 KiB Physics/NanoAODDQM
  • Checked 210 log files, 47 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@mariadalfonso
Copy link
Contributor

mariadalfonso commented Aug 3, 2022

verified the following:

  1. change of the mask to int in the storage

  2. add the new ID in miniRun3

  3. ID changed in the nano v2.1 to v2.5

  4. runs on old mini (winter 12_2 and run2)

--> 3) and 4) verified mode directly with the backport PR

Screen Shot 2022-08-03 at 16 41 10

Screen Shot 2022-08-03 at 16 40 59

@mariadalfonso
Copy link
Contributor

+xpog

@clacaputo
Copy link
Contributor

+reconstruction

  • reco differences are present only in tau.tauIDs and are in line with the content of this PR

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 4, 2022

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Aug 5, 2022

+1

@cmsbuild cmsbuild merged commit 92e4e3f into cms-sw:master Aug 5, 2022
@mbluj mbluj deleted the CMSSW_12_5_X_tau-pog_deepTau-v2p5-WPs-nano branch October 10, 2023 10:02
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.

8 participants