More code review comments

cms-sw · Jun 26, 2020 · 414a02a · 414a02a
1 parent f0f47d8
commit 414a02a
Show file tree

Hide file tree

Showing 16 changed files with 86 additions and 617 deletions.
diff --git a/L1Trigger/Phase2L1ParticleFlow/README.md b/L1Trigger/Phase2L1ParticleFlow/README.md
diff --git a/L1Trigger/Phase2L1ParticleFlow/interface/HGC3DClusterEgID.h b/L1Trigger/Phase2L1ParticleFlow/interface/HGC3DClusterEgID.h
@@ -10,7 +10,6 @@
 #include "DataFormats/L1TParticleFlow/interface/PFCluster.h"
 #include "CommonTools/Utils/interface/StringCutObjectSelector.h"
 #include "CommonTools/Utils/interface/StringObjectFunction.h"
-#include "CommonTools/MVAUtils/interface/TMVAZipReader.h"
 
 #include "TMVA/Factory.h"
 #include "TMVA/Reader.h"
@@ -21,48 +20,11 @@
 namespace l1tpf {
   class HGC3DClusterEgID {
   public:
-    HGC3DClusterEgID(const edm::ParameterSet &pset)
-        : isPUFilter_(pset.getParameter<bool>("isPUFilter")),
-          preselection_(pset.getParameter<std::string>("preselection")),
-          method_(pset.getParameter<std::string>("method")),
-          weightsFile_(pset.getParameter<std::string>("weightsFile")),
-          reader_(new TMVA::Reader()),
-          wp_(pset.getParameter<std::string>("wp")) {
-      // first create all the variables
-      for (const auto &psvar : pset.getParameter<std::vector<edm::ParameterSet>>("variables")) {
-        variables_.emplace_back(psvar.getParameter<std::string>("name"), psvar.getParameter<std::string>("value"));
-      }
-    }
+    HGC3DClusterEgID(const edm::ParameterSet &pset) ;
 
-    void prepareTMVA() {
-      // Declare the variables
-      for (auto &var : variables_)
-        var.declare(*reader_);
-      // then read the weights
-      if (weightsFile_[0] != '/' && weightsFile_[0] != '.') {
-        weightsFile_ = edm::FileInPath(weightsFile_).fullPath();
-      }
-      reco::details::loadTMVAWeights(&*reader_, method_, weightsFile_);
-    }
+    void prepareTMVA() ;
 
-    float passID(l1t::HGCalMulticluster c, l1t::PFCluster &cpf) {
-      if (preselection_(c)) {
-        for (auto &var : variables_)
-          var.fill(c);
-        float mvaOut = reader_->EvaluateMVA(method_);
-        if (isPUFilter_)
-          cpf.setEgVsPUMVAOut(mvaOut);
-        else
-          cpf.setEgVsPionMVAOut(mvaOut);
-        return (mvaOut > wp_(c) ? 1 : 0);
-      } else {
-        if (isPUFilter_)
-          cpf.setEgVsPUMVAOut(-100.0);
-        else
-          cpf.setEgVsPionMVAOut(-100.0);
-        return 0;
-      }
-    }
+    float passID(l1t::HGCalMulticluster c, l1t::PFCluster &cpf) ;
 
     std::string method() { return method_; }
 

diff --git a/L1Trigger/Phase2L1ParticleFlow/interface/ParametricResolution.h b/L1Trigger/Phase2L1ParticleFlow/interface/ParametricResolution.h
@@ -6,28 +6,30 @@
 #include <cmath>
 
 namespace l1tpf {
+
   class ParametricResolution {
   public:
+    static std::vector<float> getVFloat(const edm::ParameterSet &cpset, const std::string & name) {
+        std::vector<double> vd = cpset.getParameter<std::vector<double>>(name);
+        return std::vector<float>(vd.begin(),vd.end());
+    }
+
     ParametricResolution() {}
-    ParametricResolution(const edm::ParameterSet &cpset) {
-      std::vector<double> etaBins = cpset.getParameter<std::vector<double>>("etaBins");
-      std::vector<double> offset = cpset.getParameter<std::vector<double>>("offset");
-      std::vector<double> scale = cpset.getParameter<std::vector<double>>("scale");
-      etas.insert(etas.end(), etaBins.begin(), etaBins.end());
-      scales.insert(scales.end(), scale.begin(), scale.end());
-      offsets.insert(offsets.end(), offset.begin(), offset.end());
+    ParametricResolution(const edm::ParameterSet &cpset) :
+        etas(getVFloat(cpset,"etaBins")),
+        offsets(getVFloat(cpset,"offset")),
+        scales(getVFloat(cpset,"scale"))
+        {
       if (cpset.existsAs<std::vector<double>>("ptMin")) {
-        std::vector<double> ptMin = cpset.getParameter<std::vector<double>>("ptMin");
-        ptMins.insert(ptMins.end(), ptMin.begin(), ptMin.end());
+        ptMins = getVFloat(cpset,"ptMin");
       } else {
         float ptMin = cpset.existsAs<double>("ptMin") ? cpset.getParameter<double>("ptMin") : 0;
-        ptMins = std::vector<float>(etaBins.size(), ptMin);
+        ptMins = std::vector<float>(etas.size(), ptMin);
       }
       if (cpset.existsAs<std::vector<double>>("ptMax")) {
-        std::vector<double> ptMax = cpset.getParameter<std::vector<double>>("ptMax");
-        ptMaxs.insert(ptMaxs.end(), ptMax.begin(), ptMax.end());
+        ptMaxs = getVFloat(cpset,"ptMax");
       } else {
-        ptMaxs = std::vector<float>(etaBins.size(), 1e6);
+        ptMaxs = std::vector<float>(etas.size(), 1e6);
       }
       std::string skind = cpset.getParameter<std::string>("kind");
       if (skind == "track")

diff --git a/L1Trigger/Phase2L1ParticleFlow/interface/SimpleCalibrations.h b/L1Trigger/Phase2L1ParticleFlow/interface/SimpleCalibrations.h