Final modifications previous commit #106

MRD2F · 2018-11-13T19:45:38Z

Versioning of DPF & consistency check
Make quantized versions as default ones
General code review and cleaning.

mbluj

Thank you for the update. Please check my comment here: cms-tau-pog/RecoTauTag-TrainingFiles#5 (comment) and adjust to it if you agree. See also a few cosmetics requests - they are not mandatory but can be implemented together with other changes.

mbluj · 2018-11-14T07:54:41Z

RecoTauTag/RecoTau/python/tools/runTauIdMVA.py

+            file_name = 'RecoTauTag/TrainingFiles/data/DPFTauId/DPFIsolation_2017v0.pb'
+            self.process.dpfTau2016v0 = self.cms.EDProducer("DPFIsolation",
+                pfcands     = self.cms.InputTag('packedPFCandidates'),
+                taus 	    = self.cms.InputTag('slimmedTaus'),


Cosmetics: please adjust = together with other changes.

mbluj · 2018-11-14T07:55:33Z

RecoTauTag/RecoTau/python/tools/runTauIdMVA.py

+            file_name = 'RecoTauTag/TrainingFiles/data/DPFTauId/DPFIsolation_2017v1.pb'
+            self.process.dpfTau2016v1 = self.cms.EDProducer("DPFIsolation",
+                pfcands     = self.cms.InputTag('packedPFCandidates'),
+                taus 	    = self.cms.InputTag('slimmedTaus'),


Cosmetics: please adjust =

mbluj

Please find comments inline - cosmetics.

mbluj · 2018-11-14T12:11:59Z

RecoTauTag/RecoTau/plugins/DeepTauId.cc

@@ -249,7 +249,7 @@ class DeepTauId : public deep_tau::DeepTauBase {
        desc.add<edm::InputTag>("electrons", edm::InputTag("slimmedElectrons"));
        desc.add<edm::InputTag>("muons", edm::InputTag("slimmedMuons"));
        desc.add<edm::InputTag>("taus", edm::InputTag("slimmedTaus"));
-        desc.add<std::string>("graph_file", "RecoTauTag/TrainingFiles/data/DeepTauId/deepTau_2017v1_20L1024N_quantized.pb");
+        desc.add<std::string>("graph_file", "RecoTauTag/TrainingFiles/data/DeepTauId/deepTau_2017v1_20L1024N.pb");


quantized should be default, do you agree?

mbluj · 2018-11-14T12:12:04Z

RecoTauTag/RecoTau/plugins/DPFIsolation.cc

@@ -47,7 +47,7 @@ class DPFIsolation : public deep_tau::DeepTauBase {
        desc.add<edm::InputTag>("pfcands", edm::InputTag("packedPFCandidates"));
        desc.add<edm::InputTag>("taus", edm::InputTag("slimmedTaus"));
        desc.add<edm::InputTag>("vertices", edm::InputTag("offlineSlimmedPrimaryVertices"));
-        desc.add<std::string>("graph_file", "RecoTauTag/TrainingFiles/data/DPFTauId/DPFIsolation_2017v0_quantized.pb");
+        desc.add<std::string>("graph_file", "RecoTauTag/TrainingFiles/data/DPFTauId/DPFIsolation_2017v0.pb");


quantized should be default, do you agree?

mbluj · 2018-11-14T12:18:59Z

RecoTauTag/RecoTau/python/tools/runTauIdMVA.py

+
+
+    def getDpfTauVersion(self, file_name):
+        version_search = re.search('201[125678]v([0-9]+)[\._]', file_name)


It is quite clear, but please add a comment about expected pattern in a file name for those who might not understand regex syntax; something like this: "File name should contain a version label with data takig year (2011-2, 2015-8) and version number (vX), e.g. 2017v0."

mbluj · 2018-11-14T12:19:15Z

RecoTauTag/RecoTau/python/tools/runTauIdMVA.py

+    def getDpfTauVersion(self, file_name):
+        version_search = re.search('201[125678]v([0-9]+)[\._]', file_name)
+        if not version_search:
+            raise RuntimeError('File "{}" has an invalid name pattern. Unable to extract version number.'.format(file_name))


Please add to description of the error information on an expected version label as in a comment above.

mbluj · 2018-11-14T16:14:15Z

Tested, results as in version before final cleaning provided via #105

#105 and #106 from 94X to 104X

cms-tau-pog#105 and cms-tau-pog#106 from 94X to 104X

@perrotta

* First implementation of deep tau id. * Building dpf isolation module * Adding in v1 * Adding in runTauIDMVA for other users * making things fully reproducible * Reorganisation of configuration files: cff split to cfi and cff * Some code cleaning * adapt to cfi/cff reorganization * Review of DPF and DeepTauId code. - Defined base class for deep tau discriminators. - Removed weight files from home cms repository. Now using weights from cms-data. - Defined WP for both discriminators. Now all discriminators return the corresponding WP results. - Removed cfi files. Using fillDescriptions instead. - General code review and cleaning. * Added example of a python configuration file to produce pat::Tau collection with the new Tau-Ids * requested changes on runDeepTauIDsOnMiniAOD.py * Clean runTauIdMVA.py tool and test config to run tauIDs * Made DeepTauId and DPFIsolation thread-safe * Finish implement thread-safe requirements on DPFIsolation * Disable DPFTau_2016_v1 and issue some warnings * Remove assigning value of variable to itself * - Implemented on runTauIdMVA the option to work with new training files quantized - Added a new parameter 'version' on runTauIdMVA, used on DPFIsolation - Changes on DeepTauId to reduce memory consumption * - Implementation of global cache to avoid reloading graph for each thread and reduce the memory consuption - Creation of class DeepTauCache in DeepTauBase, in which now is created graph and session - Implementation of two new static methods inside the class DeepTauBase: initializeGlobalCache and globalEndJob. The graph and DeepTauCache object are created now inside initializeGlobalCache * Applied changes on DeepTauBase to allow load new training files using memory mapping * Implemented TauWPThreshold class. TauWPThreshold class parses WP cut string (or value) provided in the python configuration. It is needed because the use of the standard StringObjectFunction class to parse complex expression results in an extensive memory usage (> 100 MB per expression). * Remove the qm.pb input files and leaving just the quantized and the original files * -Overall, changes to improve memory usage, among these are: - Implementation of global cache to avoid reloading graph for each thread - Creation of two new static methods inside the class DeepTauBase: initializeGlobalCache and globalEndJob. The graph and DeepTauCache object are created now inside initializeGlobalCache. The memory consumption of initializeGlobalCache for the original, quantized and files that are load using memory mapping method are in the memory_usage.pdf file - Implemented configuration to use new training files quantized, and set them as default - Implementation of configuration for load files using memory mapping. In our case there wasn't any improvement, respect at the memory consumption of this method, respect the quantized files, so this is not used, but set for future training files - General code review and cleaning. * Applied style comments * Applied style comments * Applied comments * Change to be by default the original training file for deepTau, instead of the quantized * Changes regarding forward-porting DNN-related developments from the PRs #105 and #106 from 94X to 104X * Applied commets of previus PR * cleaning code * Modification in the config to work with new label in files * Applied comment about the expected format of name of training file * Fix in last commit * Applied last comments * Changes regarding forward-porting DNN-related developments from the PRs #105 and #106 from 94X to 104X * Applied @perrotta comments on 104X * Fix error * Applied comments * Applied comments * Fix merge problem * Applied a few commets * Applied more changes * Applied a few small followups * Fixed error on DPFIsolation * Update DPFIsolation.cc * - RecoTauTag/RecoTau/plugins/DeepTauId.cc: Remove ' clusterVariables 'as a class member - RecoTauTag/RecoTau/test/runDeepTauIDsOnMiniAOD.py: Update globaltag and sample * Added changes in RecoTauTag/RecoTau/python/tools/runTauIdMVA.py made in the commit 194a1d5 from the PR cms-sw#25016 * Fix error on runDeepTauIDsOnMiniAOD * Change the GT in RecoTauTag/RecoTau/test/runDeepTauIDsOnMiniAOD.py

MRD2F added 2 commits November 13, 2018 19:40

Applied commets of previus PR

c32912a

cleaning code

6df2f5b

mbluj reviewed Nov 14, 2018

View reviewed changes

kandrosov mentioned this pull request Nov 14, 2018

Update training files and labeling cms-tau-pog/RecoTauTag-TrainingFiles#5

Merged

MRD2F added 3 commits November 14, 2018 12:17

Modification in the config to work with new label in files

945df3b

Applied comment about the expected format of name of training file

554f094

Fix in last commit

e1891f7

mbluj reviewed Nov 14, 2018

View reviewed changes

Applied last comments

a9f6b7c

mbluj merged commit 7c78a26 into cms-tau-pog:CMSSW_9_4_X_tau_pog_DNNTauIDs Nov 14, 2018

MRD2F mentioned this pull request Nov 17, 2018

forward-porting DNN-related from branch cms-tau-pog:CMSSW_9_4_X_tau_pog_DNNTauIDs #107

Merged

mbluj pushed a commit that referenced this pull request Nov 20, 2018

Changes regarding forward-porting DNN-related developments from the PRs

eda5c98

#105 and #106 from 94X to 104X

MRD2F mentioned this pull request Nov 29, 2018

forward-porting DNN-related from branch cms-tau-pog:CMSSW_9_4_X_tau_pog_DNNTauIDs #111

Merged

mbluj pushed a commit that referenced this pull request Nov 30, 2018

Changes regarding forward-porting DNN-related developments from the PRs

505f4e8

#105 and #106 from 94X to 104X

mbluj pushed a commit that referenced this pull request Nov 30, 2018

Changes regarding forward-porting DNN-related developments from the PRs

df546a5

#105 and #106 from 94X to 104X

xealits pushed a commit to xealits/cmssw that referenced this pull request Jan 17, 2019

Changes regarding forward-porting DNN-related developments from the PRs

355c3ee

cms-tau-pog#105 and cms-tau-pog#106 from 94X to 104X

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Final modifications previous commit #106

Final modifications previous commit #106

MRD2F commented Nov 13, 2018

mbluj left a comment

mbluj Nov 14, 2018

mbluj Nov 14, 2018

mbluj left a comment

mbluj Nov 14, 2018

mbluj Nov 14, 2018

mbluj Nov 14, 2018

mbluj Nov 14, 2018

mbluj commented Nov 14, 2018



		def getDpfTauVersion(self, file_name):
		version_search = re.search('201[125678]v([0-9]+)[\._]', file_name)

Final modifications previous commit #106

Final modifications previous commit #106

Conversation

MRD2F commented Nov 13, 2018

mbluj left a comment

Choose a reason for hiding this comment

mbluj Nov 14, 2018

Choose a reason for hiding this comment

mbluj Nov 14, 2018

Choose a reason for hiding this comment

mbluj left a comment

Choose a reason for hiding this comment

mbluj Nov 14, 2018

Choose a reason for hiding this comment

mbluj Nov 14, 2018

Choose a reason for hiding this comment

mbluj Nov 14, 2018

Choose a reason for hiding this comment

mbluj Nov 14, 2018

Choose a reason for hiding this comment

mbluj commented Nov 14, 2018