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

Update of DeepTauID to ver. 2017v2 (backport to 10_2_X) #27148

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented Jun 7, 2019

PR description:

This PR updates DeepTauID to version 2017v2. The new version, thanks to revised structure of used DNN, fixes issues of version 2017v1 that is a size of training files and memory consumption are greatly reduced. Possibility to run previous version is maintained.

This is backport of #26796 to 10_2_X without DeepTauID in the standard MiniAOD workflow.

Details on DeepTauID performance in the original PR

Note: This PR requires datafiles from cms-data/RecoTauTag-TrainingFiles#3 although it is not necessary for official workflows and standard tests.

PR validation:

As the PR does not include anything to official workflows it was validated with a simple standalone test using RecoTauTag/RecoTau/test/runDeepTauIDsOnMiniAOD.py which requires datafiles in cms-data/RecoTauTag-TrainingFiles#3. Unit-tests should not show any differences.

if this PR is a backport please specify the original PR:

This is backport of #26796 to 10_2_X (prepared on top of 10_2_15)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 12, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/903/console Started: 2019/06/12 12:55

@perrotta
Copy link
Contributor

backport of #26796 and #27173

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@perrotta
Copy link
Contributor

In the (previous version of the) tests there was a failing comparison for the HLT DQM histo hltEle15Ele8CaloIdLTrackIdLIsoVLEtLeg1Filter_gsfEle_passFilter_HLTphi_eb for wf 136.85 RunEGamma2018A, see #27148 (comment): this looks completely un-related to me, still maybe worth being looked at by DQM in order to understand the reason for such a non-reproducibility (attn @andrius-k, @kmaeshima, @schneiml, @jfernan2, @fioriNTU)

Let see if it reproduces in the tests currently ongoing...

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 3007468
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3007276
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
  • Checked 129 log files, 14 edm output root files, 31 DQM output files

@jfernan2
Copy link
Contributor

@perrotta I guess you mean this:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_10_2_X_2019-06-11-2300+faedbf/32210/136.85_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Prompt_L1TEgDQM+HARVEST2018_L1TEgDQM/HLT_EGM_Source_Histos_hltEle15Ele8CaloIdLTrackIdLIsoVLEtLeg1Filter.html

Looking at the code changes and since there are no additional commits on top of the comparison, the change is tiny but clearly un-related... I don't have an explanation, number of entries are the same... where is the missing entry? Overflow or underflow?

@perrotta
Copy link
Contributor

Looking at the code changes and since there are no additional commits on top of the comparison, the change is tiny but clearly un-related... I don't have an explanation, number of entries are the same... where is the missing entry? Overflow or underflow?

Yes, I don't expect that such a change in the DQM plot has anything to do with this PR, I only pointed it to you in case you want to inspect it.
By the way, also in the latest test results, see #27148 (comment), the same DQM plot ends up with the same difference

@perrotta
Copy link
Contributor

+1

  • Changes since last reco signature in Update of DeepTauID to ver. 2017v2 (backport to 10_2_X) #27148 (comment) only deal with the inclusion of a minor fix to silence the static analyzer (also included in the master version of this PR) with no effect on outputs
  • Jenkins tests pass and show no differences, as expected because the new ID is not propagated to the miniAOD production sequence in this 10_2_X backport

@civanch
Copy link
Contributor

civanch commented Jun 14, 2019

+1

@cmsbuild
Copy link
Contributor

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

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 9c554bc into cms-sw:CMSSW_10_2_X Jul 11, 2019
@mbluj mbluj deleted the CMSSW_10_2_X_tau-pog_DeepTau2017v2_backport branch October 10, 2023 10:06
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

8 participants