From 0a30aa204ccb85e589582557e98c9026505cb447 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Tue, 14 Jul 2020 16:22:05 +0200 Subject: [PATCH 1/6] Use ProcessBlock in Harvesting. --- DQMServices/Components/plugins/DQMFileSaver.cc | 10 ++++------ DQMServices/Core/interface/DQMEDHarvester.h | 9 ++++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/DQMServices/Components/plugins/DQMFileSaver.cc b/DQMServices/Components/plugins/DQMFileSaver.cc index e4d09725344d7..11803cf31dd8c 100644 --- a/DQMServices/Components/plugins/DQMFileSaver.cc +++ b/DQMServices/Components/plugins/DQMFileSaver.cc @@ -25,7 +25,7 @@ namespace saverDetails { // - This includes ALCAHARVEST. TODO: check if the data written there is needed for the PCL. // This module is not used in online. This module is (hopefully?) not used at HLT. // Online and HLT use modules in DQMServices/FileIO. -class DQMFileSaver : public edm::one::EDAnalyzer { +class DQMFileSaver : public edm::one::EDAnalyzer { public: typedef dqm::legacy::DQMStore DQMStore; typedef dqm::legacy::MonitorElement MonitorElement; @@ -35,7 +35,7 @@ class DQMFileSaver : public edm::one::EDAnalyzer { void beginRun(edm::Run const &, edm::EventSetup const &) override{}; void analyze(const edm::Event &e, const edm::EventSetup &) override; void endRun(const edm::Run &, const edm::EventSetup &) override; - void endJob() override; + void endProcessBlock(const edm::ProcessBlock &) override; private: void saveForOffline(const std::string &workflow, int run, int lumi); @@ -147,10 +147,8 @@ DQMFileSaver::DQMFileSaver(const edm::ParameterSet &ps) dbe_(&*edm::Service()), nrun_(0), irun_(0) { - // Note: this is insufficient, we also need to enforce running *after* all - // DQMEDAnalyzers (a.k.a. EDProducers) in endJob. - // This is not supported in edm currently. consumesMany(); + consumesMany(); workflow_ = ps.getUntrackedParameter("workflow", workflow_); if (workflow_.empty() || workflow_[0] != '/' || *workflow_.rbegin() == '/' || std::count(workflow_.begin(), workflow_.end(), '/') != 3 || @@ -208,7 +206,7 @@ void DQMFileSaver::endRun(const edm::Run &iRun, const edm::EventSetup &) { } } -void DQMFileSaver::endJob() { +void DQMFileSaver::endProcessBlock(const edm::ProcessBlock &) { if (saveAtJobEnd_) { if (forceRunNumber_ > 0) saveForOffline(workflow_, forceRunNumber_, 0); diff --git a/DQMServices/Core/interface/DQMEDHarvester.h b/DQMServices/Core/interface/DQMEDHarvester.h index cc8e6dc50fc6a..ab6a2ed9b7f76 100644 --- a/DQMServices/Core/interface/DQMEDHarvester.h +++ b/DQMServices/Core/interface/DQMEDHarvester.h @@ -43,6 +43,7 @@ namespace edm { class DQMEDHarvester : public edm::one::EDProducer jobmegetter_; edm::GetterOfProducts runmegetter_; edm::GetterOfProducts lumimegetter_; edm::EDPutTokenT lumiToken_; edm::EDPutTokenT runToken_; + edm::EDPutTokenT jobToken_; public: DQMEDHarvester(edm::ParameterSet const &iConfig) { @@ -71,18 +74,22 @@ class DQMEDHarvester // TODO: Run/Lumi suffix should not be needed, complain to CMSSW core in case. lumiToken_ = produces(outputgeneration + "Lumi"); runToken_ = produces(outputgeneration + "Run"); + jobToken_ = produces(outputgeneration + "Job"); // Use explicitly specified inputs, but if there are none... auto inputtags = iConfig.getUntrackedParameter>("inputMEs", std::vector()); if (inputtags.empty()) { // ... use all RECO MEs. + inputtags.push_back(edm::InputTag("", inputgeneration + "Job")); inputtags.push_back(edm::InputTag("", inputgeneration + "Run")); inputtags.push_back(edm::InputTag("", inputgeneration + "Lumi")); } + jobmegetter_ = edm::GetterOfProducts(edm::VInputTagMatch(inputtags), this, edm::InProcess); runmegetter_ = edm::GetterOfProducts(edm::VInputTagMatch(inputtags), this, edm::InRun); lumimegetter_ = edm::GetterOfProducts(edm::VInputTagMatch(inputtags), this, edm::InLumi); callWhenNewProductsRegistered([this](edm::BranchDescription const &bd) { + jobmegetter_(bd); runmegetter_(bd); lumimegetter_(bd); }); @@ -135,7 +142,7 @@ class DQMEDHarvester void endRun(edm::Run const &, edm::EventSetup const &) override{}; - void endJob() final { + void endProcessBlockProduce(edm::ProcessBlock &) final { dqmstore_->meBookerGetter([this](DQMStore::IBooker &b, DQMStore::IGetter &g) { b.setScope(MonitorElementData::Scope::JOB); this->dqmEndJob(b, g); From 3f67ebfe9349c000797aa54fc41199cf23685512 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Wed, 15 Jul 2020 13:38:35 +0200 Subject: [PATCH 2/6] Use GetterOfProducts. consumesMany seems to not catch everything. --- .../Components/plugins/DQMFileSaver.cc | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/DQMServices/Components/plugins/DQMFileSaver.cc b/DQMServices/Components/plugins/DQMFileSaver.cc index 11803cf31dd8c..2390368cf2bb9 100644 --- a/DQMServices/Components/plugins/DQMFileSaver.cc +++ b/DQMServices/Components/plugins/DQMFileSaver.cc @@ -5,6 +5,9 @@ #include "FWCore/Framework/interface/Event.h" #include "FWCore/Framework/interface/Run.h" #include "FWCore/Framework/interface/LuminosityBlock.h" +#include "FWCore/Framework/interface/GetterOfProducts.h" +#include "FWCore/Framework/interface/ProcessMatch.h" +#include "FWCore/Framework/interface/LuminosityBlock.h" #include "FWCore/ParameterSet/interface/ParameterSet.h" #include "FWCore/Version/interface/GetReleaseVersion.h" #include "FWCore/ServiceRegistry/interface/Service.h" @@ -60,6 +63,11 @@ class DQMFileSaver : public edm::one::EDAnalyzer jobmegetter_; + edm::GetterOfProducts runmegetter_; }; //-------------------------------------------------------- @@ -146,9 +154,15 @@ DQMFileSaver::DQMFileSaver(const edm::ParameterSet &ps) fileBaseName_(""), dbe_(&*edm::Service()), nrun_(0), - irun_(0) { - consumesMany(); - consumesMany(); + irun_(0), + // Abuse ProcessMatch as a "match all". + jobmegetter_(edm::GetterOfProducts(edm::ProcessMatch("*"), this, edm::InProcess)), + runmegetter_(edm::GetterOfProducts(edm::ProcessMatch("*"), this, edm::InRun)) { + callWhenNewProductsRegistered([this](edm::BranchDescription const &bd) { + this->jobmegetter_(bd); + this->runmegetter_(bd); + }); + workflow_ = ps.getUntrackedParameter("workflow", workflow_); if (workflow_.empty() || workflow_[0] != '/' || *workflow_.rbegin() == '/' || std::count(workflow_.begin(), workflow_.end(), '/') != 3 || From 552baf2db112cf4ac8db5add8164d4cd91eae676 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Thu, 16 Jul 2020 16:46:31 +0200 Subject: [PATCH 3/6] Remove some endJob methods. endJob can no longer be used for DQM work, even in legacy modules. At least in production sequences, where the output would no longer be saved by DQMFileSaver. This is not a very aggressive removal; doing non-DQM work is fine, as well as setups where a module calls DQMStore::save itself. But I removed a lot of empty endJob to avoid confusion by people trying to use them. --- .../SiPixelQuality/plugins/SiPixelStatusHarvester.cc | 3 --- .../SiPixelQuality/plugins/SiPixelStatusHarvester.h | 1 - .../SiStripChannelGain/interface/SiStripGainsPCLWorker.h | 1 - .../SiStripChannelGain/src/SiStripGainsPCLWorker.cc | 3 --- DQMOffline/CalibTracker/src/StatisticsFilter.cc | 4 ---- .../Trigger/interface/DQMOfflineHLTEventInfoClient.h | 3 --- DQMOffline/Trigger/interface/HLTInclusiveVBFClient.h | 2 -- .../Trigger/plugins/DQMOfflineHLTEventInfoClient.cc | 5 ----- DQMOffline/Trigger/plugins/HLTInclusiveVBFClient.cc | 4 ---- DQMOffline/Trigger/plugins/HLTOverallSummary.cc | 8 -------- DQMServices/Components/plugins/DQMDaqInfo.cc | 2 -- DQMServices/Components/plugins/DQMDaqInfo.h | 1 - DQMServices/Components/plugins/DQMLumiMonitor.cc | 1 - DQMServices/Components/plugins/DQMLumiMonitor.h | 1 - DQMServices/Components/plugins/DQMMessageLoggerClient.cc | 3 --- DQMServices/Components/plugins/DQMMessageLoggerClient.h | 1 - DQMServices/Components/plugins/EDMtoMEConverter.h | 2 -- DQMServices/Components/plugins/MEtoEDMConverter.cc | 2 -- DQMServices/Components/plugins/MEtoEDMConverter.h | 1 - Validation/RecoTau/plugins/DQMHistNormalizer.cc | 1 - Validation/RecoTau/plugins/DQMHistPlotter.h | 1 - 21 files changed, 50 deletions(-) diff --git a/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.cc b/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.cc index 1ef2084ca1ca9..48ea04ec37fa0 100644 --- a/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.cc +++ b/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.cc @@ -97,9 +97,6 @@ SiPixelStatusHarvester::~SiPixelStatusHarvester() {} //-------------------------------------------------------------------------------------------------- void SiPixelStatusHarvester::beginJob() {} -//-------------------------------------------------------------------------------------------------- -void SiPixelStatusHarvester::endJob() {} - //-------------------------------------------------------------------------------------------------- void SiPixelStatusHarvester::bookHistograms(DQMStore::IBooker& iBooker, edm::Run const&, diff --git a/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.h b/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.h index f74f567df59d4..c2fb7a437125f 100644 --- a/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.h +++ b/CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.h @@ -43,7 +43,6 @@ class SiPixelStatusHarvester : public DQMOneEDAnalyzer dqm_tag_; diff --git a/CalibTracker/SiStripChannelGain/src/SiStripGainsPCLWorker.cc b/CalibTracker/SiStripChannelGain/src/SiStripGainsPCLWorker.cc index 664c3ef1001da..989668e292a33 100644 --- a/CalibTracker/SiStripChannelGain/src/SiStripGainsPCLWorker.cc +++ b/CalibTracker/SiStripChannelGain/src/SiStripGainsPCLWorker.cc @@ -541,9 +541,6 @@ void SiStripGainsPCLWorker::checkBookAPVColls(const TrackerGeometry* bareTkGeomP } //if (!bareTkGeomPtr_) ... } -//********************************************************************************// -void SiStripGainsPCLWorker::endJob() {} - //********************************************************************************// void SiStripGainsPCLWorker::fillDescriptions(edm::ConfigurationDescriptions& descriptions) { edm::ParameterSetDescription desc; diff --git a/DQMOffline/CalibTracker/src/StatisticsFilter.cc b/DQMOffline/CalibTracker/src/StatisticsFilter.cc index d77389d4e620e..41e5c9d83be32 100644 --- a/DQMOffline/CalibTracker/src/StatisticsFilter.cc +++ b/DQMOffline/CalibTracker/src/StatisticsFilter.cc @@ -48,7 +48,6 @@ class StatisticsFilter : public edm::EDFilter { private: void beginJob() override; bool filter(edm::Event&, const edm::EventSetup&) override; - void endJob() override; // ----------member data --------------------------- @@ -121,8 +120,5 @@ bool StatisticsFilter::filter(edm::Event& iEvent, const edm::EventSetup& iSetup) // ------------ method called once each job just before starting event loop ------------ void StatisticsFilter::beginJob() {} -// ------------ method called once each job just after ending the event loop ------------ -void StatisticsFilter::endJob() {} - //define this as a plug-in DEFINE_FWK_MODULE(StatisticsFilter); diff --git a/DQMOffline/Trigger/interface/DQMOfflineHLTEventInfoClient.h b/DQMOffline/Trigger/interface/DQMOfflineHLTEventInfoClient.h index 2ece911fa096c..454795f5c96e9 100644 --- a/DQMOffline/Trigger/interface/DQMOfflineHLTEventInfoClient.h +++ b/DQMOffline/Trigger/interface/DQMOfflineHLTEventInfoClient.h @@ -41,9 +41,6 @@ class DQMOfflineHLTEventInfoClient : public edm::EDAnalyzer { /// EndRun void endRun(const edm::Run& r, const edm::EventSetup& c) override; - /// Endjob - void endJob() override; - private: void initialize(); edm::ParameterSet parameters_; diff --git a/DQMOffline/Trigger/interface/HLTInclusiveVBFClient.h b/DQMOffline/Trigger/interface/HLTInclusiveVBFClient.h index de29d80bc8172..e67ec928346ba 100644 --- a/DQMOffline/Trigger/interface/HLTInclusiveVBFClient.h +++ b/DQMOffline/Trigger/interface/HLTInclusiveVBFClient.h @@ -54,8 +54,6 @@ class HLTInclusiveVBFClient : public edm::EDAnalyzer { explicit HLTInclusiveVBFClient(const edm::ParameterSet&); ~HLTInclusiveVBFClient() override; - void beginJob() override; - void endJob() override; void beginRun(const edm::Run& run, const edm::EventSetup& c) override; void endRun(const edm::Run& run, const edm::EventSetup& c) override; void analyze(const edm::Event&, const edm::EventSetup&) override; diff --git a/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc b/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc index 36662571823e7..8f9eec5b4438e 100644 --- a/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc +++ b/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc @@ -58,9 +58,6 @@ class DQMOfflineHLTEventInfoClient: public edm::EDAnalyzer { /// EndRun void endRun(const edm::Run& r, const edm::EventSetup& c); - /// Endjob - void endJob(); - private: void initialize(); @@ -252,5 +249,3 @@ void DQMOfflineHLTEventInfoClient::endRun(const Run& r, const EventSetup& contex CertificationSummaryMap_->setBinContent(1, 6, tauValue); //Tau } -//-------------------------------------------------------- -void DQMOfflineHLTEventInfoClient::endJob() {} diff --git a/DQMOffline/Trigger/plugins/HLTInclusiveVBFClient.cc b/DQMOffline/Trigger/plugins/HLTInclusiveVBFClient.cc index dff95c33fadaa..707d98e681ae3 100644 --- a/DQMOffline/Trigger/plugins/HLTInclusiveVBFClient.cc +++ b/DQMOffline/Trigger/plugins/HLTInclusiveVBFClient.cc @@ -37,8 +37,6 @@ HLTInclusiveVBFClient::HLTInclusiveVBFClient(const edm::ParameterSet& iConfig) : HLTInclusiveVBFClient::~HLTInclusiveVBFClient() = default; -void HLTInclusiveVBFClient::beginJob() {} - void HLTInclusiveVBFClient::beginRun(const edm::Run& r, const edm::EventSetup& context) {} void HLTInclusiveVBFClient::analyze(const edm::Event& iEvent, const edm::EventSetup& iSetup) {} @@ -49,8 +47,6 @@ void HLTInclusiveVBFClient::endLuminosityBlock(const edm::LuminosityBlock& lumiS void HLTInclusiveVBFClient::endRun(const edm::Run& r, const edm::EventSetup& context) {} -void HLTInclusiveVBFClient::endJob() {} - void HLTInclusiveVBFClient::runClient_() { if (!dbe_) return; //we dont have the DQMStore so we cant do anything diff --git a/DQMOffline/Trigger/plugins/HLTOverallSummary.cc b/DQMOffline/Trigger/plugins/HLTOverallSummary.cc index 5c4dc024fef5f..61bd5ab4f059a 100644 --- a/DQMOffline/Trigger/plugins/HLTOverallSummary.cc +++ b/DQMOffline/Trigger/plugins/HLTOverallSummary.cc @@ -56,9 +56,7 @@ class HLTOverallSummary : public edm::EDAnalyzer { explicit HLTOverallSummary(const edm::ParameterSet& pset); ~HLTOverallSummary() override; - void beginJob() override; void analyze(const edm::Event&, const edm::EventSetup&) override; - void endJob() override; void beginRun(const edm::Run&, const edm::EventSetup&) override; void endRun(const edm::Run&, const edm::EventSetup&) override; @@ -102,12 +100,6 @@ void HLTOverallSummary::analyze(const edm::Event& iEvent, const edm::EventSetup& LogInfo("HLTMuonVal") << ">>> Analyze (HLTOverallSummary) <<<" << std::endl; } -// ------------ method called once each job just before starting event loop ------------ -void HLTOverallSummary::beginJob() {} - -// ------------ method called once each job just after ending the event loop ------------ -void HLTOverallSummary::endJob() {} - // ------------ method called just before starting a new run ------------ void HLTOverallSummary::beginRun(const edm::Run& run, const edm::EventSetup& c) { using namespace edm; diff --git a/DQMServices/Components/plugins/DQMDaqInfo.cc b/DQMServices/Components/plugins/DQMDaqInfo.cc index 0bdcb22f28b7f..bec2abbca7516 100644 --- a/DQMServices/Components/plugins/DQMDaqInfo.cc +++ b/DQMServices/Components/plugins/DQMDaqInfo.cc @@ -137,6 +137,4 @@ void DQMDaqInfo::beginJob() { NumberOfFeds[L1T] = L1TRange.second - L1TRange.first + 1; } -void DQMDaqInfo::endJob() {} - void DQMDaqInfo::analyze(const edm::Event& iEvent, const edm::EventSetup& iSetup) {} diff --git a/DQMServices/Components/plugins/DQMDaqInfo.h b/DQMServices/Components/plugins/DQMDaqInfo.h index 42ffe230b1edb..1e8e5079bb26b 100644 --- a/DQMServices/Components/plugins/DQMDaqInfo.h +++ b/DQMServices/Components/plugins/DQMDaqInfo.h @@ -53,7 +53,6 @@ class DQMDaqInfo : public edm::EDAnalyzer { void beginJob() override; void beginLuminosityBlock(const edm::LuminosityBlock&, const edm::EventSetup&) override; void analyze(const edm::Event&, const edm::EventSetup&) override; - void endJob() override; DQMStore* dbe_; diff --git a/DQMServices/Components/plugins/DQMLumiMonitor.cc b/DQMServices/Components/plugins/DQMLumiMonitor.cc index c6a5096cac806..b29afe78883b5 100644 --- a/DQMServices/Components/plugins/DQMLumiMonitor.cc +++ b/DQMServices/Components/plugins/DQMLumiMonitor.cc @@ -142,7 +142,6 @@ void DQMLumiMonitor::endLuminosityBlock(edm::LuminosityBlock const& lumiBlock, e void DQMLumiMonitor::endRun(edm::Run const& iRun, edm::EventSetup const& iSetup) {} -void DQMLumiMonitor::endJob() {} // Define this as a plug-in #include "FWCore/Framework/interface/MakerMacros.h" DEFINE_FWK_MODULE(DQMLumiMonitor); diff --git a/DQMServices/Components/plugins/DQMLumiMonitor.h b/DQMServices/Components/plugins/DQMLumiMonitor.h index 06a73d7985b91..387bb90df8ecf 100644 --- a/DQMServices/Components/plugins/DQMLumiMonitor.h +++ b/DQMServices/Components/plugins/DQMLumiMonitor.h @@ -40,7 +40,6 @@ class DQMLumiMonitor : public edm::EDAnalyzer { void analyze(edm::Event const& iEvent, edm::EventSetup const& iSetup) override; void endLuminosityBlock(edm::LuminosityBlock const& lumiSeg, edm::EventSetup const& eSetup) override; void endRun(edm::Run const& iRun, edm::EventSetup const& iSetup) override; - void endJob() override; private: void bookHistograms(); diff --git a/DQMServices/Components/plugins/DQMMessageLoggerClient.cc b/DQMServices/Components/plugins/DQMMessageLoggerClient.cc index 234ce190c2b22..a381c33a8184c 100644 --- a/DQMServices/Components/plugins/DQMMessageLoggerClient.cc +++ b/DQMServices/Components/plugins/DQMMessageLoggerClient.cc @@ -170,6 +170,3 @@ void DQMMessageLoggerClient::fillHistograms() { void DQMMessageLoggerClient::endRun(const Run& r, const EventSetup& es) { fillHistograms(); } -void DQMMessageLoggerClient::endJob() { - //LogTrace(metname)<<"[DQMMessageLoggerClient] EndJob"; -} diff --git a/DQMServices/Components/plugins/DQMMessageLoggerClient.h b/DQMServices/Components/plugins/DQMMessageLoggerClient.h index 1ea0607243d10..5162373521e9f 100644 --- a/DQMServices/Components/plugins/DQMMessageLoggerClient.h +++ b/DQMServices/Components/plugins/DQMMessageLoggerClient.h @@ -29,7 +29,6 @@ class DQMMessageLoggerClient : public edm::EDAnalyzer { // Save the histos void endRun(const edm::Run &, const edm::EventSetup &) override; - void endJob() override; private: void fillHistograms(); diff --git a/DQMServices/Components/plugins/EDMtoMEConverter.h b/DQMServices/Components/plugins/EDMtoMEConverter.h index 360c8adbfadfb..d1a6f102bcd45 100644 --- a/DQMServices/Components/plugins/EDMtoMEConverter.h +++ b/DQMServices/Components/plugins/EDMtoMEConverter.h @@ -60,8 +60,6 @@ class EDMtoMEConverter : public edm::one::EDProducer MEtoEDMConverter::globalBeginRun(edm::Run const& iRun, const edm::EventSetup& iSetup) const { return std::shared_ptr(); diff --git a/DQMServices/Components/plugins/MEtoEDMConverter.h b/DQMServices/Components/plugins/MEtoEDMConverter.h index dd838de1c9cfb..418563c1607d3 100644 --- a/DQMServices/Components/plugins/MEtoEDMConverter.h +++ b/DQMServices/Components/plugins/MEtoEDMConverter.h @@ -68,7 +68,6 @@ class MEtoEDMConverter : public edm::one::EDProducer, explicit MEtoEDMConverter(const edm::ParameterSet&); ~MEtoEDMConverter() override; void beginJob() override; - void endJob() override; void produce(edm::Event&, const edm::EventSetup&) override; std::shared_ptr globalBeginRun(edm::Run const&, const edm::EventSetup&) const override; void globalEndRun(edm::Run const&, const edm::EventSetup&) override; diff --git a/Validation/RecoTau/plugins/DQMHistNormalizer.cc b/Validation/RecoTau/plugins/DQMHistNormalizer.cc index afe72cabf3c5e..f38575ef9048e 100644 --- a/Validation/RecoTau/plugins/DQMHistNormalizer.cc +++ b/Validation/RecoTau/plugins/DQMHistNormalizer.cc @@ -35,7 +35,6 @@ class DQMHistNormalizer : public edm::EDAnalyzer { explicit DQMHistNormalizer(const edm::ParameterSet&); ~DQMHistNormalizer() override; void analyze(const edm::Event&, const edm::EventSetup&) override; - void endJob() override {} void endRun(const edm::Run& r, const edm::EventSetup& c) override; private: diff --git a/Validation/RecoTau/plugins/DQMHistPlotter.h b/Validation/RecoTau/plugins/DQMHistPlotter.h index ebb50dfc9d587..1804c143371d2 100644 --- a/Validation/RecoTau/plugins/DQMHistPlotter.h +++ b/Validation/RecoTau/plugins/DQMHistPlotter.h @@ -157,7 +157,6 @@ class TauDQMHistPlotter : public edm::EDAnalyzer { explicit TauDQMHistPlotter(const edm::ParameterSet&); ~TauDQMHistPlotter() override; void analyze(const edm::Event&, const edm::EventSetup&) override; - void endJob() override {} void endRun(const edm::Run& r, const edm::EventSetup& c) override; private: From 6f9ca2720b2c5b377ad4d2515b250b4a81ab3176 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Thu, 16 Jul 2020 17:53:40 +0200 Subject: [PATCH 4/6] Remove endJob harvesting support for legacy harvesters. It seems it was not really used anyways. --- DQMServices/Core/README.md | 7 ++++--- DQMServices/Demo/test/TestHarvester.cc | 7 ------- DQMServices/Demo/test/runtests.sh | 4 +++- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/DQMServices/Core/README.md b/DQMServices/Core/README.md index 821e661b94568..161e660428c43 100644 --- a/DQMServices/Core/README.md +++ b/DQMServices/Core/README.md @@ -161,10 +161,11 @@ To clarify how (per-lumi) saving and merging of histograms happens, we introduce - When harvesting, `reScope` is set to `RUN` (or `JOB`). Now, MEs saved with scope `LUMI` will be switched to scope `RUN` (or `JOB`) and merged. The harvesting modules can observe increasing statistics in the histogram as a run is processed (like in online DQM). - For multi-run harvesting, `reScope` is set to `JOB`. Now, even `RUN` histograms are merged. This is the default, since it also works for today's single-run harvesting jobs. - Currently, EDM does not allow `JOB` products, and therefore output modules cannot save at the end of the `JOB` scope. The only file format where `JOB` scope is supported is the legacy `TDirectory` `DQMFileSaver`. - - This means that harvesting jobs can _only_ save in legacy format, since most harvesting logic runs at `endJob`. + - We use `ProcessBlock` products to model the dataflow that traditionally happened at `endJob`. + - Legacy modules cannot do `JOB` level harvesting: their code in `endJob` only runs after the output file is saved. Some code remains there, and it can still work if output is saved by a different means than `DQMFileSaver`. + - This means that harvesting jobs can _only_ save in legacy format, since most harvesting logic runs at `dqmEndJob` (a.k.a. `endProcessBlock`. Output modules don't support `ProcessBlock`s yet. - For the same reason, _all_ harvesting jobs use `reScope = JOB` today. However, for single-run harvesting jobs, some logic in the `DQMFileSaver` still attaches the run number of the (only) run processed to the output file (this can and will go wrong if there is more than one run in a "normal" harvesting job). - This also means that apart from the run number on the output file, "normal" and multi-run harvesting jobs are identical. - - Once EDM gains the ability to handle units of data that are more than one run, it would make sense to revise the harvesting setup to run all harvesting code outside of `endJob`, in the new transition, and harvesting could switch to outputting DQMIO instead of legacy output files (the DQMIO file format would need to be extended as well). Harvesting jobs are always processed sequentially, like the data was taken: runs and lumisections are processed in increasing order. This is implemented in `DQMRootSource`. @@ -174,7 +175,7 @@ DQM promises that all data dependencies across the `DQMStore` are visible to EDM - `DQMGenerationReco` is produced by `DQMEDAnalyzer`s, which consume only "normal" (non-DQM) products. - `DQMGenerationHarvesting` is produced by `DQMEDHarvester`s, which consume (by default) all `DQMGenerationReco` tokens. This allows all old code to work without explicitly declaring dependencies. `DQMEDHarvester`s provide a mechanism to consume more specific tokens, including `DQMGenerationHarvesting` tokens from other harvesters. - `DQMGenerationQTest` is produced only by the `QualityTester` module (of which we typically have many instances). The `QualityTester` has fully configurable dependencies to allow the more complicated setups sometimes required in harvesting, but typically consumes `DQMGenerationHarvesting`. -- There is a hidden dependency between end run and end job in harvesting: Many harvesters effectively depend on `DQMGenerationQTest` products (the QTest results) and do not specify that, but things still work because they do their work in `endJob`. +- There is a hidden dependency between end run and end process block in harvesting: Many harvesters effectively depend on `DQMGenerationQTest` products (the QTest results) and do not specify that, but things still work because they do their work in `dqmEndJob` (a.k.a. `endProcessBlock`). #### Local `MonitorElement`s vs. global `MonitorElement`s diff --git a/DQMServices/Demo/test/TestHarvester.cc b/DQMServices/Demo/test/TestHarvester.cc index 95d3a8fc05dbf..c88028506aa49 100644 --- a/DQMServices/Demo/test/TestHarvester.cc +++ b/DQMServices/Demo/test/TestHarvester.cc @@ -73,13 +73,6 @@ class TestLegacyHarvester : public edm::EDAnalyzer { out->Fill(whathappened); } - void endJob() override { - edm::Service store; - whathappened += "endJob() "; - store->setCurrentFolder(folder_); - MonitorElement *out = store->bookString("harvestingsummary", "missing"); - out->Fill(whathappened); - } void endLuminosityBlock(edm::LuminosityBlock const &lumi, edm::EventSetup const &) override { whathappened += "endLumi(" + std::to_string(lumi.run()) + "," + std::to_string(lumi.luminosityBlock()) + ") "; } diff --git a/DQMServices/Demo/test/runtests.sh b/DQMServices/Demo/test/runtests.sh index 66a66de5da3f5..ebe3ba7617490 100755 --- a/DQMServices/Demo/test/runtests.sh +++ b/DQMServices/Demo/test/runtests.sh @@ -125,7 +125,9 @@ cmsRun $LOCAL_TEST_DIR/run_analyzers_cfg.py numberEventsInRun=300 numberEventsIn # 10. Try running some harvesting modules and check if their output makes it out. # Note that we pass the files out-of order here; the DQMIO input should sort them. cmsRun $LOCAL_TEST_DIR/run_harvesters_cfg.py inputFiles=part1.root inputFiles=part3.root inputFiles=part2.root legacyoutput=True -[ 2 = $(rootlist DQM_V0001_R000000001__Harvesting__DQMTests__DQMIO.root | grep -c 's=beginRun(1) endLumi(1,1) endLumi(1,2) endLumi(1,3) endRun(1) endJob() ') ] +[ 1 = $(rootlist DQM_V0001_R000000001__Harvesting__DQMTests__DQMIO.root | grep -c 's=beginRun(1) endLumi(1,1) endLumi(1,2) endLumi(1,3) endRun(1) endJob() ') ] +# The legacy harvester can only do per-run harvesting. +[ 2 = $(rootlist DQM_V0001_R000000001__Harvesting__DQMTests__DQMIO.root | grep -c 's=beginRun(1) endLumi(1,1) endLumi(1,2) endLumi(1,3) endRun(1) ') ] # 11. Try MEtoEDM and EDMtoME. cmsRun $LOCAL_TEST_DIR/run_analyzers_cfg.py outfile=metoedm.root numberEventsInRun=100 numberEventsInLuminosityBlock=20 nEvents=100 metoedmoutput=True From 2e93d41658b7ca57b7a697aed8f9e4979e58e4cc Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Thu, 16 Jul 2020 18:20:18 +0200 Subject: [PATCH 5/6] Code-format. --- DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc | 1 - DQMServices/Components/plugins/DQMMessageLoggerClient.cc | 1 - 2 files changed, 2 deletions(-) diff --git a/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc b/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc index 8f9eec5b4438e..697de6f0f4e19 100644 --- a/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc +++ b/DQMOffline/Trigger/plugins/DQMOfflineHLTEventInfoClient.cc @@ -248,4 +248,3 @@ void DQMOfflineHLTEventInfoClient::endRun(const Run& r, const EventSetup& contex CertificationSummaryMap_->setBinContent(1, 5, 1); //BJet CertificationSummaryMap_->setBinContent(1, 6, tauValue); //Tau } - diff --git a/DQMServices/Components/plugins/DQMMessageLoggerClient.cc b/DQMServices/Components/plugins/DQMMessageLoggerClient.cc index a381c33a8184c..0c1fe8f1187c0 100644 --- a/DQMServices/Components/plugins/DQMMessageLoggerClient.cc +++ b/DQMServices/Components/plugins/DQMMessageLoggerClient.cc @@ -169,4 +169,3 @@ void DQMMessageLoggerClient::fillHistograms() { } void DQMMessageLoggerClient::endRun(const Run& r, const EventSetup& es) { fillHistograms(); } - From 4df3118908d082bc70dc54537d6f488848ecce74 Mon Sep 17 00:00:00 2001 From: Marcel Andre Schneider Date: Thu, 23 Jul 2020 18:36:11 +0200 Subject: [PATCH 6/6] Add dependencies for SiPixelPhase1Summary. --- .../python/SiPixelPhase1Summary_cfi.py | 9 +++++++++ DQM/SiPixelPhase1Summary/src/SiPixelPhase1Summary.cc | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/DQM/SiPixelPhase1Summary/python/SiPixelPhase1Summary_cfi.py b/DQM/SiPixelPhase1Summary/python/SiPixelPhase1Summary_cfi.py index 617860252a028..bd28caa06393a 100644 --- a/DQM/SiPixelPhase1Summary/python/SiPixelPhase1Summary_cfi.py +++ b/DQM/SiPixelPhase1Summary/python/SiPixelPhase1Summary_cfi.py @@ -9,6 +9,9 @@ TopFolderName = cms.string('PixelPhase1/Phase1_MechanicalView/'), RunOnEndLumi = cms.bool(True), RunOnEndJob = cms.bool(True), + # schedule this module to run *after* the QTests. + inputGeneration = cms.untracked.string('DQMGenerationQTest'), + outputGeneration = cms.untracked.string('DQMGenerationSummary'), SummaryMaps = cms.VPSet( cms.PSet( MapName = cms.string("Digi"), @@ -39,6 +42,9 @@ TopFolderName = cms.string('PixelPhase1/Phase1_MechanicalView/'), RunOnEndLumi = cms.bool(False), RunOnEndJob = cms.bool(True), + # schedule this module to run *after* the QTests. + inputGeneration = cms.untracked.string('DQMGenerationQTest'), + outputGeneration = cms.untracked.string('DQMGenerationSummary'), SummaryMaps = cms.VPSet( cms.PSet( MapName = cms.string("Digi"), @@ -69,6 +75,9 @@ TopFolderName = cms.string('PixelPhase1/Phase1_MechanicalView/'), RunOnEndLumi = cms.bool(False), RunOnEndJob = cms.bool(True), + # schedule this module to run *after* the QTests. + inputGeneration = cms.untracked.string('DQMGenerationQTest'), + outputGeneration = cms.untracked.string('DQMGenerationSummary'), SummaryMaps = cms.VPSet( cms.PSet( MapName = cms.string("Digi"), diff --git a/DQM/SiPixelPhase1Summary/src/SiPixelPhase1Summary.cc b/DQM/SiPixelPhase1Summary/src/SiPixelPhase1Summary.cc index fb7fdc85f3931..f5d83890ba7db 100644 --- a/DQM/SiPixelPhase1Summary/src/SiPixelPhase1Summary.cc +++ b/DQM/SiPixelPhase1Summary/src/SiPixelPhase1Summary.cc @@ -46,7 +46,8 @@ using namespace std; using namespace edm; -SiPixelPhase1Summary::SiPixelPhase1Summary(const edm::ParameterSet& iConfig) : conf_(iConfig), firstLumi(true) { +SiPixelPhase1Summary::SiPixelPhase1Summary(const edm::ParameterSet& iConfig) + : DQMEDHarvester(iConfig), conf_(iConfig), firstLumi(true) { LogInfo("PixelDQM") << "SiPixelPhase1Summary::SiPixelPhase1Summary: Got DQM BackEnd interface" << endl; topFolderName_ = conf_.getParameter("TopFolderName"); runOnEndLumi_ = conf_.getParameter("RunOnEndLumi");