Preparations of deep tau ID for hlt phase2 (backport to CMSSW_11_1_X) #32831

swozniewski · 2021-02-05T17:05:21Z

PR description:

Backports #30761 #31744 #32676 #32838 in order to have DeepTauID developments available for phase2 HLT TDR

Prepare DeepTauID such that it can run on AOD inputs and be used within HLT sequences.
#30761 was some central clean up that interferes with the other two PRs. For convenience and consistent state of the code it is included.

PR validation:

code checks, format passed
unit tests passed
limited matrix tests passed

- input tokens for candidates and taus now use base classes - taus and candidates accessed by dynamic_cast and edm::View - is_online boolean in fillDescriptions - Electron and muon collections empty if is_online - Changed some quantities with functions that calculate quantity differently depending on the input

Removed 2 leftover comments

- Ran "scram build code-checks" and "scram build code-format" - Implemented the new container format. For now the indices of the discriminators are given as inputs in the config file. I will look into using provenance - Switched order of the if statements (for is_online and version checks) in the getPredictions function - turned tau_index to const

…nc to edm::Handle instead of the vector. The way it was before worked in pre3 but switching to pre5 broke that.

- Change TauRefType to edm::RefToBase<reco::BaseTau>

…ching from provenance.

…scriminators into static const

- Added, removed and changed some option for the basicDiscriminators in the deepTauAtHLT.py config file to be more in line with RecoTauTag/Configuration/HPSPFTaus_cff.py

…that the list of working-points is empty

…olation pT-sum The HGCal work-around is motivated by this presentation: https://indico.cern.ch/event/954050/contributions/4013705/attachments/2103331/3536699/HLTTauTrigger_status_2020Sep16.pdf

- added option to save all input variables to JSON file (this feature allows to compare the input variables in C++ vs python/during the DeepTau training)

…x for accessing the grid of low-level variables

…d" to true in ParameterSet description (true restores the old behaviour of the code)

…Konstantin

cmsbuild · 2021-02-05T20:42:01Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9586d4/12741/summary.html
COMMIT: 541726e
CMSSW: CMSSW_11_1_X_2021-01-31-0000/slc7_amd64_gcc820
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/32831/12741/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2784828
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2784777
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 31 edm output root files, 36 DQM output files

slava77 · 2021-02-05T22:58:51Z

RecoTauTag/RecoTau/src/PositionAtECalEntranceComputer.cc

-reco::Candidate::Point PositionAtECalEntranceComputer::operator()(const reco::Candidate* particle, bool& success) const {
+reco::Candidate::Point PositionAtECalEntranceComputer::operator()(const reco::Candidate* particle,
+                                                                  bool& success) const {


this is not a part of the listed PRs used for backport.
Apparently, the change is coming from
https://github.com/cms-sw/cmssw/pull/30571/files#diff-225356b5230d327db61da9f1a9a6514e03ec42126c806f2a46fc54c97a7a64ebL19-R20

it seems acceptable to include though.

I think it is rather this commit 9457485 which is probably not backported but where I applied the code-format at some point. Sorry for the confusion!

cmsbuild · 2021-02-08T10:43:27Z

Pull request #32831 was updated. @perrotta, @Martin-Grunewald, @slava77, @cmsbuild, @fwyzard, @jpata can you please check and sign again.

perrotta · 2021-02-08T11:31:34Z

please test

cmsbuild · 2021-02-08T14:25:48Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9586d4/12758/summary.html
COMMIT: 6834218
CMSSW: CMSSW_11_1_X_2021-02-07-0000/slc7_amd64_gcc820
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32831/12758/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2784828
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2784776
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 31 edm output root files, 36 DQM output files

fwyzard · 2021-02-09T13:44:26Z

+hlt

The only HLT specific change is the addition of a customisation function.

fwyzard · 2021-02-09T13:47:54Z

@slava77 @perrotta is this good for RECO ?

slava77 · 2021-02-09T17:52:17Z

+1

for #32831 6834218

the backport is appropriate under a full no-change policy (no changes in the default setup are expected)
- the diffs are in agreement with the PR description (removed unnessecary argument of EventSetup from getPredictions function #30761 -> Change DeepTauID modules so they can also process RECO input for HLT purposes #31744 -> Further preparations of deep tau ID for hlt phase2 #32676 -> use non-static lambda in DeepTauId::CellGrid::tryGetCellIndex #32838 )
- comparing 11_1_X -> master in the modified files, except for RecoTauTag/RecoTau/python/tools/runTauIdMVA.py, there are technical modifications from Revision of tau quality cut configuration #30364 and [Clang tidy] Apply checks for reconstruction #30805 which should be OK to skip, and Principal::getProvenance can no longer trigger unscheduled execution #32614 is 113X-specific and does not apply
- runTauIdMVA.py in this PR has just an addition of is_online parameter, and it seems OK to skip other changes in this file as unrelated to deepTau
jenkins tests pass and comparisons with the baseline show no (relevant) differences

cmsbuild · 2021-02-09T17:52:37Z

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_3_X is complete. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2021-02-10T15:52:58Z

+1

JamminJones and others added 30 commits January 18, 2021 11:08

removed unnessecary argument of EventSetup from getPredictions function

6041fed

Update DeepTauId.cc

0d7ee8a

Removed 2 leftover comments

Added customization file used for testing module at RECO level

dc11b7f

Changed type of electron_collection and muon_collection in lightLepFu…

01c2673

…nc to edm::Handle instead of the vector. The way it was before worked in pre3 but switching to pre5 broke that.

- Implement Prediscriminators

31b7b15

- Change TauRefType to edm::RefToBase<reco::BaseTau>

Switch discriminator indices for RECO from configuration input to mat…

59cc574

…ching from provenance.

Translated map with indices for the basic discriminators to enum

e30bdf6

Cleanup of leftover lines from testing

37e02d2

Made stringFromDiscriminator map and the vectors defining required di…

29dc4c1

…scriminators into static const

Once again forgot to remove a print line from testing

71587f8

- Changed some member names to be according to the naming conventions

ffdd965

- Added, removed and changed some option for the basicDiscriminators in the deepTauAtHLT.py config file to be more in line with RecoTauTag/Configuration/HPSPFTaus_cff.py

apply code formatting

d7acd1a

remove redundant const qualifiers

95ba68e

move deepTauAtHLT.py to RecoTauTag/HLTProducers

2c367d2

Make changes requested in PR cms-sw#31744

9a6c59d

fix code format

7ef5bb2

Make changes requested in PR cms-sw#31744 (#143)

66081f5

fix input pfCand_muon_dz_sig

bafab6e

pass WPList as ponter instead of by reference, to allow for the case …

4c11e4a

…that the list of working-points is empty

added option to apply HGCal work-around when computing the charged is…

788a035

…olation pT-sum The HGCal work-around is motivated by this presentation: https://indico.cern.ch/event/954050/contributions/4013705/attachments/2103331/3536699/HLTTauTrigger_status_2020Sep16.pdf

- improved checks of input variables

4b4a1f3

- added option to save all input variables to JSON file (this feature allows to compare the input variables in C++ vs python/during the DeepTau training)

added disable_CellIndex_workaround parameter to enable/disable bug-fi…

73158e9

…x for accessing the grid of low-level variables

set "disable_hcalFraction_workaround" and"disable_CellIndex_workaroun…

5bda512

…d" to true in ParameterSet description (true restores the old behaviour of the code)

code polishing as suggested by Sebastian

204a818

more code polishing, following comments by Sebastian

36c54c8

set disable_hcalFraction_workaround to false by default

5239bb2

inverted definition of disable_CellIndex_workaround, as requested by …

03552b5

…Konstantin

apply code checks and automated format

4430e19

cmsbuild added tests-approved and removed tests-started labels Feb 5, 2021

slava77 reviewed Feb 5, 2021

View reviewed changes

use non-static lambda in DeepTauId::CellGrid::tryGetCellIndex

6834218

cmsbuild added tests-pending and removed tests-approved labels Feb 8, 2021

cmsbuild added tests-started and removed tests-pending labels Feb 8, 2021

cmsbuild added tests-approved and removed tests-started labels Feb 8, 2021

cmsbuild added hlt-approved and removed hlt-pending labels Feb 9, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Feb 9, 2021

cmsbuild added orp-approved and removed orp-pending labels Feb 10, 2021

cmsbuild merged commit d7e5468 into cms-sw:CMSSW_11_1_X Feb 10, 2021

fwyzard mentioned this pull request May 11, 2021

Implement Phase2 HLT Taus and L1HPSTaus fwyzard/cmssw#16

Closed

mbluj deleted the CMSSW_11_1_X_tau-pog_DeepTauIDforHLT_backport branch October 10, 2023 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preparations of deep tau ID for hlt phase2 (backport to CMSSW_11_1_X) #32831

Preparations of deep tau ID for hlt phase2 (backport to CMSSW_11_1_X) #32831

swozniewski commented Feb 5, 2021 •

edited

cmsbuild commented Feb 5, 2021

slava77 Feb 5, 2021

swozniewski Feb 8, 2021

cmsbuild commented Feb 8, 2021

perrotta commented Feb 8, 2021

cmsbuild commented Feb 8, 2021

fwyzard commented Feb 9, 2021

fwyzard commented Feb 9, 2021

slava77 commented Feb 9, 2021

cmsbuild commented Feb 9, 2021

qliphy commented Feb 10, 2021

Preparations of deep tau ID for hlt phase2 (backport to CMSSW_11_1_X) #32831

Preparations of deep tau ID for hlt phase2 (backport to CMSSW_11_1_X) #32831

Conversation

swozniewski commented Feb 5, 2021 • edited

PR description:

PR validation:

cmsbuild commented Feb 5, 2021

Comparison Summary

slava77 Feb 5, 2021

Choose a reason for hiding this comment

swozniewski Feb 8, 2021

Choose a reason for hiding this comment

cmsbuild commented Feb 8, 2021

perrotta commented Feb 8, 2021

cmsbuild commented Feb 8, 2021

Comparison Summary

fwyzard commented Feb 9, 2021

fwyzard commented Feb 9, 2021

slava77 commented Feb 9, 2021

cmsbuild commented Feb 9, 2021

qliphy commented Feb 10, 2021

swozniewski commented Feb 5, 2021 •

edited