adding new PLVLoose/Tight definition
Trying to add new isolation WPs based on PromptLeptonVeto score, and the low-pt version of it (we call it LowPtPLV). There are major updates because we needed to add LowPtPLT in IsolationSelection package, as they are not currently calculated at derivation level.
Presentation about the final WP definition and its performance: https://indico.cern.ch/event/851820/contributions/3581694/attachments/1917246/3170461/IFF_20190930_PLVWP.pdf
Discussion on LowPtPLV: https://indico.cern.ch/event/848448/contributions/3564603/attachments/1908395/3152628/20190916_akatsuka_PLT.pdf
Sorry the codes are still quite dirty, submitting this MR to start the discussion... tagging @maklein @oducu @shion @rustem
Merge request reports
Activity
Hi @sakatsuk, could you also please add ATLAS Robot as Developer to your fork? (https://atlassoftwaredocs.web.cern.ch/gittutorial/gitlab-fork/#add-your-friendly-build-bot)
Thanks, Tadej (L1)
WARNING: big files (>100K) are found in the changeset 1692K in file PhysicsAnalysis/AnalysisCommon/IsolationSelection/data/TMVAClassification_BDT_Muon_LowPtPromptLeptonTagger_20181001.weights.xml 1436K in file PhysicsAnalysis/AnalysisCommon/IsolationSelection/data/TMVAClassification_BDT_Electron_LowPtPromptLeptonTagger_20181001.weights.xmladded 21.2 Analysis review-pending-level-1 labels
WARNING: big files (>100K) are found in the changeset 1692K in file PhysicsAnalysis/AnalysisCommon/IsolationSelection/data/TMVAClassification_BDT_Muon_LowPtPromptLeptonTagger_20181001.weights.xml 1436K in file PhysicsAnalysis/AnalysisCommon/IsolationSelection/data/TMVAClassification_BDT_Electron_LowPtPromptLeptonTagger_20181001.weights.xml CI Result SUCCESSAthDerivation externals cmake make required tests optional tests Full details available on this CI monitor view
AthDerivation: number of compilation errors 0, warnings 12
For experts only: Jenkins output [CI-MERGE-REQUEST 40889] CI Result SUCCESSAnalysisBase AnalysisTop AthAnalysis externals cmake make required tests optional tests Full details available on this CI monitor view
AnalysisBase: number of compilation errors 0, warnings 4
AnalysisTop: number of compilation errors 0, warnings 4
AthAnalysis: number of compilation errors 0, warnings 15
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 3923]- Resolved by Shunichi Akatsuka
added review-user-action-required label and removed review-pending-level-1 label
9 #include <AsgTools/AnaToolHandle.h> 10 #include <IsolationSelection/IIsolationLowPtPLVTool.h> 11 #include <vector> 12 #include <string> 13 14 // TMVA 15 #include "TMVA/Reader.h" 16 17 namespace CP { 18 class IsolationLowPtPLVTool: public asg::AsgTool, public virtual IIsolationLowPtPLVTool { 19 20 public: 21 IsolationLowPtPLVTool(const std::string& name); 22 ASG_TOOL_CLASS(IsolationLowPtPLVTool, IIsolationLowPtPLVTool) 23 virtual StatusCode initialize(); 24 StatusCode AugmentPLV(xAOD::IParticle* Particles) const; What interface class?
This "tool" class is just all kinds of wrong.
You shouldn't make a class inherit fromasg::AsgTool
if it doesn't implement a proper pure-virtual interface. And this class really doesn't.At first I thought that this may be a (scary) pattern already present in this package. But thankfully that doesn't seem to be the case.
I also don't understand how the changes in
IsolationSelectionTool.cxx
relate to this new class. Though that may be just my own lack of understanding.Normally at this point I'd ask for a talk/discussion about this in an ASG meeting. But with those absent at the moment, I guess we have to follow up like this. Which will take a bit longer than just discussing live...
- Resolved by Shunichi Akatsuka
- Resolved by Shunichi Akatsuka
- Resolved by Chaowaroj Wanotayaroj