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
Electron MVA V2 and General Restructuring of Egamma IDs - Once again #23700
Electron MVA V2 and General Restructuring of Egamma IDs - Once again #23700
Conversation
All event based variables are now handeled by the new helper class Hardcoding of variables and clips removed for Fall17 Generalized Fall17Iso and Fall17NoIso Configurable categories and start to move Spring16 MVA Dedicated variable files for Spring16 and Fall17 Removed Spring16 C++ code All year specific C++ code removed Small improvements Cleared some inconsistencies in variable definitions. All IDs now validated. Restructured VID python code a bit New MVAVariableManager helper class Forgot to actually add the class go git The raw BDT score is now stored as well Spring15 PhotonMVA uses now MVAVariableManager Almost there for photon MVA All Run2 Photon MVA IDs use now the new PhotonMVAEstimator class Add variable files Rewrote some python, ElectronID cuts on raw MWA values and autodetection of gzipped weight files Implemented V2 MV Ele ID Cut values are now found with parser as well Include ntupizer for training cleaned up a bit Training Ntuplizer finished Use auto generatetd config files Finalized ntuplizer Auto updated Training ntuplizer Updated training ntuplizer Fix in training ntuplizer fixed wp90 iso V1 parameters Fix Added MVA output Validation ntuplizer for photons Bugfix Improved exception handling in GBRForestToors
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23700/5356 |
A new Pull Request was created by @guitargeek (Jonas Rembser) for master. It involves the following packages: PhysicsTools/NanoAOD @perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test workflow 134.707,136.7722,1325.9, 10001.0 with cms-sw/cmsdist#4056 |
The tests are being triggered in jenkins. |
-1 Tested at: e72ad05 You can see the results of the tests here: I found follow errors while testing this PR Failed tests: UnitTests RelVals
I found errors in the following unit tests: ---> test runtestRecoEgammaElectronIdentification had ERRORS
When I ran the RelVals I found an error in the following worklfows: runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step3_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log134.707 step3 runTheMatrix-results/134.707_RunSingleEl2015B+RunSingleEl2015B+HLTDR2_50ns+RECODR2_50nsreHLT_HIPM+HARVESTDR2/step3_RunSingleEl2015B+RunSingleEl2015B+HLTDR2_50ns+RECODR2_50nsreHLT_HIPM+HARVESTDR2.log136.7722 step2 runTheMatrix-results/136.7722_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X/step2_RunJetHT2016H_nano+RunJetHT2016H_nano+NANOEDM2016_80X+HARVESTNANOAOD2016_80X.log9.0 step3 runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step3_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log135.4 step3 runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step3_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log136.731 step3 runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2/step3_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2.log136.7611 step2 runTheMatrix-results/136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log136.8311 step2 runTheMatrix-results/136.8311_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017/step2_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017.log25.0 step3 runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT/step3_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT.log136.788 step3 runTheMatrix-results/136.788_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017/step3_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017.log1325.7 step2 runTheMatrix-results/1325.7_TTbar_13_94Xv2NanoAODINPUT+TTbar_13_94Xv2NanoAODINPUT+NANOEDMMC2017_94XMiniAODv2+HARVESTNANOAODMC2017_94XMiniAODv2/step2_TTbar_13_94Xv2NanoAODINPUT+TTbar_13_94Xv2NanoAODINPUT+NANOEDMMC2017_94XMiniAODv2+HARVESTNANOAODMC2017_94XMiniAODv2.log1325.9 step2 runTheMatrix-results/1325.9_TTbar_13_92XNanoAODINPUT+TTbar_13_92XNanoAODINPUT+NANOEDMMC2017_92X+HARVESTNANOAODMC2017_92X/step2_TTbar_13_92XNanoAODINPUT+TTbar_13_92XNanoAODINPUT+NANOEDMMC2017_92X+HARVESTNANOAODMC2017_92X.log1306.0 step3 runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step3_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log136.85 step3 runTheMatrix-results/136.85_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Prompt_L1TEgDQM+HARVEST2018_L1TEgDQM/step3_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Prompt_L1TEgDQM+HARVEST2018_L1TEgDQM.log1330.0 step3 runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM/step3_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM.log10001.0 step3 runTheMatrix-results/10001.0_SingleElectronPt10+SingleElectronPt10_pythia8_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017/step3_SingleElectronPt10+SingleElectronPt10_pythia8_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017.log10042.0 step3 runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017/step3_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017.log25202.0 step3 runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step3_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log10024.0 step3 runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017.log11624.0 step3 runTheMatrix-results/11624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+HARVESTFull_2019+ALCAFull_2019/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+HARVESTFull_2019+ALCAFull_2019.log10824.0 step3 runTheMatrix-results/10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018.log10224.0 step3 runTheMatrix-results/10224.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU.log20034.0 step3 runTheMatrix-results/20034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17.log20434.0 step3 runTheMatrix-results/20434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D19_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D19+RecoFullGlobal_2023D19+HARVESTFullGlobal_2023D19/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D19_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D19+RecoFullGlobal_2023D19+HARVESTFullGlobal_2023D19.log21234.0 step3 runTheMatrix-results/21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21.log250202.181 step4 runTheMatrix-results/250202.181_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25/step4_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25.log |
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
@cmsbuild please test workflow 134.707,136.7722,1325.9,1344.0,10001.0 with cms-sw/cmsdist#4161 |
The tests are being triggered in jenkins. |
In the meanwhile I run 500 events with wf 1344.0 with no crashes or other issues... |
Thanks a lot Andrea thats be very reassuring! |
Comparison job queued. |
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
+1
|
+1 |
merge |
Should we worry about the FileInPathErrors in the IB? All the RelVals are failing there, and due to a file that wasn't even touched. I hope it's not severe. If at all it's a problem with the externals. I checked:
There is no ElectronIdentification anymore... |
@guitargeek that was a problem in the build, that has been solved. |
@@ -12,19 +12,15 @@ class AnyMVAEstimatorRun2Base { | |||
|
|||
public: | |||
// Constructor, destructor | |||
AnyMVAEstimatorRun2Base(const edm::ParameterSet& conf) : _conf(conf) {} | |||
AnyMVAEstimatorRun2Base(const edm::ParameterSet& conf) : conf(conf) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity: is there any possible ambiguity from this line?
@Dr15Jones ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it compilers, there is no confusion :). However the member data name conf
does violate CMS naming policy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought only private members are named with a training underscore. Ok, next time also the public ones. But conf
should actually be private, I just didn't want to change too much.
All event based variables are now handeled by the new helper class Hardcoding of variables and clips removed for Fall17 Generalized Fall17Iso and Fall17NoIso Configurable categories and start to move Spring16 MVA Dedicated variable files for Spring16 and Fall17 Removed Spring16 C++ code All year specific C++ code removed Small improvements Cleared some inconsistencies in variable definitions. All IDs now validated. Restructured VID python code a bit New MVAVariableManager helper class Forgot to actually add the class go git The raw BDT score is now stored as well Spring15 PhotonMVA uses now MVAVariableManager Almost there for photon MVA All Run2 Photon MVA IDs use now the new PhotonMVAEstimator class Add variable files Rewrote some python, ElectronID cuts on raw MWA values and autodetection of gzipped weight files Implemented V2 MV Ele ID Cut values are now found with parser as well Include ntupizer for training cleaned up a bit Training Ntuplizer finished Use auto generatetd config files Finalized ntuplizer Auto updated Training ntuplizer Updated training ntuplizer Fix in training ntuplizer fixed wp90 iso V1 parameters Fix Added MVA output Validation ntuplizer for photons Bugfix Improved exception handling in GBRForestToors Changed egammaObjectModificationsInMiniAOD_cff.py to comply with new config files Add two lines in miniAOD_tools.py to cope with ElectronMVAVariableHelper Fixed Fall17V1 variables Addressed comments following visual code inspection Simplified code Included Fall17 MVAs in MiniAOD and NanoAOD Added electronMVAVariableHelper to relevant processes Revisited casting of candidates to electrons/photons Fixup - prefer dynamic_cast if possible Corrected the second C style cast in the photons Avoid undefined category for photons Changed category not defined error to warning Changed mvaValue if category not found to -999 Fixed possible memory leak Drop MVAObjectCache in MVAValueMapProducer Commit after some code review
Electron MVA V2 (as in cms-sw#23700 cms-sw#23746 cms-sw#23767) All event based variables are now handeled by the new helper class Hardcoding of variables and clips removed for Fall17 Generalized Fall17Iso and Fall17NoIso Configurable categories and start to move Spring16 MVA Dedicated variable files for Spring16 and Fall17 Removed Spring16 C++ code All year specific C++ code removed Small improvements Cleared some inconsistencies in variable definitions. All IDs now validated. Restructured VID python code a bit New MVAVariableManager helper class Forgot to actually add the class go git The raw BDT score is now stored as well Spring15 PhotonMVA uses now MVAVariableManager Almost there for photon MVA All Run2 Photon MVA IDs use now the new PhotonMVAEstimator class Add variable files Rewrote some python, ElectronID cuts on raw MWA values and autodetection of gzipped weight files Implemented V2 MV Ele ID Cut values are now found with parser as well Include ntupizer for training cleaned up a bit Training Ntuplizer finished Use auto generatetd config files Finalized ntuplizer Auto updated Training ntuplizer Updated training ntuplizer Fix in training ntuplizer fixed wp90 iso V1 parameters Fix Added MVA output Validation ntuplizer for photons Bugfix Improved exception handling in GBRForestToors Changed egammaObjectModificationsInMiniAOD_cff.py to comply with new config files Add two lines in miniAOD_tools.py to cope with ElectronMVAVariableHelper Fixed Fall17V1 variables Addressed comments following visual code inspection Simplified code Included Fall17 MVAs in MiniAOD and NanoAOD Added electronMVAVariableHelper to relevant processes Revisited casting of candidates to electrons/photons Fixup - prefer dynamic_cast if possible Corrected the second C style cast in the photons Avoid undefined category for photons Changed category not defined error to warning Changed mvaValue if category not found to -999 Fixed possible memory leak Drop MVAObjectCache in MVAValueMapProducer Commit after some code review Comment out something so branch compiles Added weightfiles Some code cleaning (as in ID part of cms-sw#23743) Adapt GBRForestTools exception handling to 94X Fall17 cutbased Electron ID V2 (as in cms-sw#23477) Added Photon MVA weight files Technical Egamma MVA improvements (as in cms-sw#24131) Fall17_94X_v2 MVA Photon ID Fixing mvaClassName in egammaObjectModificationsInMiniAOD_cff.py Inclusion of V2 IDs in MiniAOD fix worst charged iso variable adding photon MVA V2 to the pat::Photon Fixed copy-paste typo in Spring15 weight files New MultiTokens (as in cms-sw#24312 cms-sw#24423) cache AOD/miniAOD values to speed up repeated computations Egamma IDs back to global cache (as in cms-sw#25101) Cutbased photonID Fall17 V2 update phoID v2 cutbased Renamed cut based ID and put in MiniAOD Modified PhotonNtuplizer to have genMatch information Added variables from Fall17 ID to PhotonMVANtuplizer New MVAVariableHelper for extra MVA variables Moved Photon MVA to new interface adapted constructors to run MVA estimator in python fwlite ElectronMVA can be used in FWlite Egamma Python wrapper class for Electron MVAs Got rid of ElectronMVAVariableHelper Adapted PhotonMVANtuplizer Combined Electron Ntuplizer config files Fixing problems detected while testing exposing category in FWLite to be able to apply working point later on infrastructure to take working points in python+fwlite. implemented for noiso v2 only so far working points for the other mvas. somthing fishy with GP v1 Spring16 WPs with logistic transform
This is a retake on #23473, which has caused problems in some tests (see #23692 and #23693).
It is relevant for @Sam-Harper, @lsoffi, @michelif, sorry for the spam guys...
So these workflows failed last time (https://cms-sw.github.io/relvalLogDetail.html#slc6_amd64_gcc630;CMSSW_10_2_X_2018-06-26-1100):
I was not able to test all of them myself due to DAS errors, but I could run number 1. 2. 4. and 5.
About the external requirement: