Merge pull request #30698 from schneiml/dqm-use-processblock

DQM: Use ProcessBlock in Harvesting

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&,

CalibTracker/SiPixelQuality/plugins/SiPixelStatusHarvester.h

@@ -43,7 +43,6 @@ class SiPixelStatusHarvester : public DQMOneEDAnalyzer<edm::one::WatchLuminosity
 
   // Operations
   void beginJob() override;
-  void endJob() override;
   void bookHistograms(DQMStore::IBooker& iBooker, edm::Run const&, const edm::EventSetup&) final;
   void dqmEndRun(const edm::Run&, const edm::EventSetup&) final;
   void analyze(const edm::Event& iEvent, const edm::EventSetup&) final;

CalibTracker/SiStripChannelGain/interface/SiStripGainsPCLWorker.h

@@ -87,7 +87,6 @@ class SiStripGainsPCLWorker : public DQMGlobalEDAnalyzer<APVGain::APVGainHistogr
 private:
   void beginJob() override;
   void dqmBeginRun(edm::Run const &, edm::EventSetup const &, APVGain::APVGainHistograms &) const override;
-  void endJob() override;
   void checkBookAPVColls(const TrackerGeometry *bareTkGeomPtr, APVGain::APVGainHistograms &histograms) const;
 
   std::vector<std::string> dqm_tag_;

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;

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"),

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<std::string>("TopFolderName");
   runOnEndLumi_ = conf_.getParameter<bool>("RunOnEndLumi");

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);

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_;

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;

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();
@@ -251,6 +248,3 @@ void DQMOfflineHLTEventInfoClient::endRun(const Run& r, const EventSetup& contex
   CertificationSummaryMap_->setBinContent(1, 5, 1);              //BJet
   CertificationSummaryMap_->setBinContent(1, 6, tauValue);       //Tau
 }
-
-//--------------------------------------------------------
-void DQMOfflineHLTEventInfoClient::endJob() {}

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

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;

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) {}

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_;
 

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"
@@ -25,7 +28,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<edm::one::WatchRuns> {
+class DQMFileSaver : public edm::one::EDAnalyzer<edm::one::WatchRuns, edm::WatchProcessBlock> {
 public:
   typedef dqm::legacy::DQMStore DQMStore;
   typedef dqm::legacy::MonitorElement MonitorElement;
@@ -35,7 +38,7 @@ class DQMFileSaver : public edm::one::EDAnalyzer<edm::one::WatchRuns> {
   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);
@@ -60,6 +63,11 @@ class DQMFileSaver : public edm::one::EDAnalyzer<edm::one::WatchRuns> {
 
   // needed only for the harvesting step when saving in the endJob
   int irun_;
+
+  // We want to consume all DQMTokens for runs and jobs. But consumesMany is
+  // confused by the labels, so we sue these getters.
+  edm::GetterOfProducts<DQMToken> jobmegetter_;
+  edm::GetterOfProducts<DQMToken> runmegetter_;
 };
 
 //--------------------------------------------------------
@@ -146,11 +154,15 @@ DQMFileSaver::DQMFileSaver(const edm::ParameterSet &ps)
       fileBaseName_(""),
       dbe_(&*edm::Service<DQMStore>()),
       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<DQMToken, edm::InRun>();
+      irun_(0),
+      // Abuse ProcessMatch as a "match all".
+      jobmegetter_(edm::GetterOfProducts<DQMToken>(edm::ProcessMatch("*"), this, edm::InProcess)),
+      runmegetter_(edm::GetterOfProducts<DQMToken>(edm::ProcessMatch("*"), this, edm::InRun)) {
+  callWhenNewProductsRegistered([this](edm::BranchDescription const &bd) {
+    this->jobmegetter_(bd);
+    this->runmegetter_(bd);
+  });
+
   workflow_ = ps.getUntrackedParameter<std::string>("workflow", workflow_);
   if (workflow_.empty() || workflow_[0] != '/' || *workflow_.rbegin() == '/' ||
       std::count(workflow_.begin(), workflow_.end(), '/') != 3 ||
@@ -208,7 +220,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);

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);

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();

DQMServices/Components/plugins/DQMMessageLoggerClient.cc

@@ -169,7 +169,3 @@ void DQMMessageLoggerClient::fillHistograms() {
 }
 
 void DQMMessageLoggerClient::endRun(const Run& r, const EventSetup& es) { fillHistograms(); }
-
-void DQMMessageLoggerClient::endJob() {
-  //LogTrace(metname)<<"[DQMMessageLoggerClient] EndJob";
-}

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();

DQMServices/Components/plugins/EDMtoMEConverter.h

@@ -60,8 +60,6 @@ class EDMtoMEConverter : public edm::one::EDProducer<edm::one::WatchRuns,
   explicit EDMtoMEConverter(const edm::ParameterSet&);
   ~EDMtoMEConverter() override;
 
-  void beginJob() final{};
-  void endJob() final{};
   void beginRun(const edm::Run&, const edm::EventSetup&) final{};
   void endRun(const edm::Run&, const edm::EventSetup&) final{};
   void beginLuminosityBlock(const edm::LuminosityBlock&, const edm::EventSetup&) final{};

DQMServices/Components/plugins/MEtoEDMConverter.cc

@@ -83,8 +83,6 @@ MEtoEDMConverter::~MEtoEDMConverter() = default;
 
 void MEtoEDMConverter::beginJob() {}
 
-void MEtoEDMConverter::endJob() {}
-
 std::shared_ptr<meedm::Void> MEtoEDMConverter::globalBeginRun(edm::Run const& iRun,
                                                               const edm::EventSetup& iSetup) const {
   return std::shared_ptr<meedm::Void>();

DQMServices/Components/plugins/MEtoEDMConverter.h

@@ -68,7 +68,6 @@ class MEtoEDMConverter : public edm::one::EDProducer<edm::RunCache<meedm::Void>,
   explicit MEtoEDMConverter(const edm::ParameterSet&);
   ~MEtoEDMConverter() override;
   void beginJob() override;
-  void endJob() override;
   void produce(edm::Event&, const edm::EventSetup&) override;
   std::shared_ptr<meedm::Void> globalBeginRun(edm::Run const&, const edm::EventSetup&) const override;
   void globalEndRun(edm::Run const&, const edm::EventSetup&) override;

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