Properly handle Run product merging in LHESource

FWCore/Sources/interface/ProducerSourceFromFiles.h

@@ -33,7 +33,8 @@ namespace edm {
     using FromFiles::incrementFileIndex;
 
   private:
-    using FromFiles::fileIndex;
+    size_t fileIndex() const override;
+
   };
 }
 #endif

FWCore/Sources/src/ProducerSourceFromFiles.cc

@@ -15,5 +15,10 @@ namespace edm {
     ProducerSourceBase::fillDescription(desc);
     FromFiles::fillDescription(desc);
   }
+
+  size_t
+  ProducerSourceFromFiles::fileIndex() const {
+    return FromFiles::fileIndex();
+  }
 }
 

GeneratorInterface/LHEInterface/interface/LHEReader.h

@@ -43,6 +43,7 @@ class LHEReader {
 	std::unique_ptr<XMLDocument>	curDoc;
 	boost::shared_ptr<LHERunInfo>	curRunInfo;
 	std::unique_ptr<XMLHandler>	handler;
+	std::shared_ptr<void>           platform;
 };
 
 } // namespace lhef

GeneratorInterface/LHEInterface/plugins/LHESource.cc

@@ -5,7 +5,6 @@
 #include <memory>
 
 #include <boost/bind.hpp>
-#include <boost/ptr_container/ptr_deque.hpp>
 
 #include "FWCore/Framework/interface/InputSourceMacros.h"
 #include "FWCore/Framework/interface/MakerMacros.h"
@@ -36,14 +35,12 @@ using namespace lhef;
 LHESource::LHESource(const edm::ParameterSet &params,
                      const edm::InputSourceDescription &desc) :
   ProducerSourceFromFiles(params, desc, false),
-  reader(new LHEReader(fileNames(), params.getUntrackedParameter<unsigned int>("skipEvents", 0))),
-  wasMerged(false),
+  reader_(new LHEReader(fileNames(), params.getUntrackedParameter<unsigned int>("skipEvents", 0))),
   lheProvenanceHelper_(edm::TypeID(typeid(LHEEventProduct)), edm::TypeID(typeid(LHERunInfoProduct)), productRegistryUpdate()),
-  phid_(),
-  runPrincipal_()
+  phid_()
 {
   nextEvent();
-  lheProvenanceHelper_.lheAugment(runInfo.get());
+  lheProvenanceHelper_.lheAugment(nullptr);
   // Initialize metadata, and save the process history ID for use every event.
   phid_ = lheProvenanceHelper_.lheInit(processHistoryRegistryForUpdate());
 
@@ -58,68 +55,67 @@ LHESource::~LHESource()
 
 void LHESource::endJob()
 {
-  reader.reset();
+  reader_.reset();
 }
 
 void LHESource::nextEvent()
 {
-  if (partonLevel) {
+  if (partonLevel_) {
     return;
   }
 
   bool newFileOpened;
   do {
     newFileOpened = false;
-    partonLevel = reader->next(&newFileOpened);
-  } while (newFileOpened && !partonLevel);
+    partonLevel_ = reader_->next(&newFileOpened);
+    if(newFileOpened) {
+      incrementFileIndex();
+    }
+  } while (newFileOpened && !partonLevel_);
 
-  if (!partonLevel) {
+  if (!partonLevel_) {
     return;
   }
 
-  boost::shared_ptr<LHERunInfo> runInfoThis = partonLevel->getRunInfo();
-  if (runInfoThis != runInfoLast) {
-    runInfo = runInfoThis;
-    runInfoLast = runInfoThis;
-  }
-  if (runInfo) {
-    std::auto_ptr<LHERunInfoProduct> product(
-                                             new LHERunInfoProduct(*runInfo->getHEPRUP()));
-    std::for_each(runInfo->getHeaders().begin(),
-                  runInfo->getHeaders().end(),
-                  boost::bind(
-                              &LHERunInfoProduct::addHeader,
-                              product.get(), _1));
-    std::for_each(runInfo->getComments().begin(),
-                  runInfo->getComments().end(),
-                  boost::bind(&LHERunInfoProduct::addComment,
-                              product.get(), _1));
-
-    if (!runInfoProducts.empty()) {
-      if (runInfoProducts.front().mergeProduct(*product)) {
-        if (!wasMerged) {
-          runInfoProducts.pop_front();
-          runInfoProducts.push_front(product);
-          wasMerged = true;
-        }
-      } else {
-        lheProvenanceHelper_.lheAugment(runInfo.get());
+  auto runInfoThis = partonLevel_->getRunInfo();
+  if (runInfoThis != runInfoLast_) {
+    runInfoLast_ = runInfoThis;
+    std::unique_ptr<LHERunInfoProduct> product(
+                                             new LHERunInfoProduct(*runInfoThis->getHEPRUP()));
+    fillRunInfoProduct(*runInfoThis, *product);
+
+    if (runInfoProductLast_) {
+      if (!runInfoProductLast_->mergeProduct(*product)) {
+        //cannot be merged so must start new Run
+        runInfoProductLast_ = std::move(product);
+        lheProvenanceHelper_.lheAugment(runInfoThis.get());
         // Initialize metadata, and save the process history ID for use every event.
         phid_ = lheProvenanceHelper_.lheInit(processHistoryRegistryForUpdate());
         resetRunAuxiliary();
       }
+    } else {
+      runInfoProductLast_ = std::move(product);
     }
+  }
+}
 
-    runInfo.reset();
+void 
+LHESource::fillRunInfoProduct(lhef::LHERunInfo const& iInfo, LHERunInfoProduct& oProduct) {
+  for(auto const& h: iInfo.getHeaders()) {
+    oProduct.addHeader(h);
+  }
+  for(auto const& c: iInfo.getComments()) {
+    oProduct.addComment(c);
   }
 }
 
-// This is the only way we can now access the run principal.
+
 void
 LHESource::readRun_(edm::RunPrincipal& runPrincipal) {
   runAuxiliary()->setProcessHistoryID(phid_);
   runPrincipal.fillRunPrincipal(processHistoryRegistryForUpdate());
-  runPrincipal_ = &runPrincipal;
+
+  putRunInfoProduct(runPrincipal);
 }
 
 void
@@ -128,52 +124,21 @@ LHESource::readLuminosityBlock_(edm::LuminosityBlockPrincipal& lumiPrincipal) {
   lumiPrincipal.fillLuminosityBlockPrincipal(processHistoryRegistryForUpdate());
 }
 
-void LHESource::beginRun(edm::Run&)
-{
-  if (runInfoLast) {
-    runInfo = runInfoLast;
-
-    std::unique_ptr<LHERunInfoProduct> product(
-                                               new LHERunInfoProduct(*runInfo->getHEPRUP()));
-    std::for_each(runInfo->getHeaders().begin(),
-                  runInfo->getHeaders().end(),
-                  boost::bind(
-                              &LHERunInfoProduct::addHeader,
-                              product.get(), _1));
-    std::for_each(runInfo->getComments().begin(),
-                  runInfo->getComments().end(),
-                  boost::bind(&LHERunInfoProduct::addComment,
-                              product.get(), _1));
-
-    // keep a copy around in case of merging
-    runInfoProducts.push_back(new LHERunInfoProduct(*product));
-    wasMerged = false;
-
-    std::unique_ptr<edm::WrapperBase> rdp(new edm::Wrapper<LHERunInfoProduct>(std::move(product)));
-    runPrincipal_->put(lheProvenanceHelper_.runProductBranchDescription_, std::move(rdp));
-
-    runInfo.reset();
-  }
-}
-
-void LHESource::endRun(edm::Run&)
-{
-  if (!runInfoProducts.empty()) {
-    std::unique_ptr<LHERunInfoProduct> product(
-                                               runInfoProducts.pop_front().release());
+void LHESource::putRunInfoProduct(edm::RunPrincipal& iRunPrincipal) {
+  if (runInfoProductLast_) {
+    auto product = std::make_unique<LHERunInfoProduct>(*runInfoProductLast_);
     std::unique_ptr<edm::WrapperBase> rdp(new edm::Wrapper<LHERunInfoProduct>(std::move(product)));
-    runPrincipal_->put(lheProvenanceHelper_.runProductBranchDescription_, std::move(rdp));
+    iRunPrincipal.put(lheProvenanceHelper_.runProductBranchDescription_, std::move(rdp));
   }
-  runPrincipal_ = nullptr;
 }
 
 bool LHESource::setRunAndEventInfo(edm::EventID&, edm::TimeValue_t&, edm::EventAuxiliary::ExperimentType&)
 {
   nextEvent();
-  if (!partonLevel) {
+  if (!partonLevel_) {
     // We just finished an input file. See if there is another.
     nextEvent();
-    if (!partonLevel) {
+    if (!partonLevel_) {
       // No more input files.
       return false;
     }
@@ -189,28 +154,28 @@ LHESource::readEvent_(edm::EventPrincipal& eventPrincipal) {
   eventPrincipal.fillEventPrincipal(aux, processHistoryRegistryForUpdate());
 
   std::unique_ptr<LHEEventProduct> product(
-                                           new LHEEventProduct(*partonLevel->getHEPEUP(),
-                                                               partonLevel->originalXWGTUP())
+                                           new LHEEventProduct(*partonLevel_->getHEPEUP(),
+                                                               partonLevel_->originalXWGTUP())
                                            );
-  if (partonLevel->getPDF()) {
-    product->setPDF(*partonLevel->getPDF());
+  if (partonLevel_->getPDF()) {
+    product->setPDF(*partonLevel_->getPDF());
   }
-  std::for_each(partonLevel->weights().begin(),
-                partonLevel->weights().end(),
+  std::for_each(partonLevel_->weights().begin(),
+                partonLevel_->weights().end(),
                 boost::bind(&LHEEventProduct::addWeight,
                             product.get(), _1));
-  product->setScales(partonLevel->scales());
-  product->setNpLO(partonLevel->npLO());
-  product->setNpNLO(partonLevel->npNLO());
-  std::for_each(partonLevel->getComments().begin(),
-                partonLevel->getComments().end(),
+  product->setScales(partonLevel_->scales());
+  product->setNpLO(partonLevel_->npLO());
+  product->setNpNLO(partonLevel_->npNLO());
+  std::for_each(partonLevel_->getComments().begin(),
+                partonLevel_->getComments().end(),
                 boost::bind(&LHEEventProduct::addComment,
                             product.get(), _1));
 
   std::unique_ptr<edm::WrapperBase> edp(new edm::Wrapper<LHEEventProduct>(std::move(product)));
   eventPrincipal.put(lheProvenanceHelper_.eventProductBranchDescription_, std::move(edp), lheProvenanceHelper_.eventProductProvenance_);
 
-  partonLevel.reset();
+  partonLevel_.reset();
 
   resetEventCached();
 }

GeneratorInterface/LHEInterface/plugins/LHESource.h

@@ -3,9 +3,9 @@
 
 #include <memory>
 
-#include <boost/shared_ptr.hpp>
-#include <boost/ptr_container/ptr_deque.hpp>
+#include <deque>
 
+#include <boost/shared_ptr.hpp>
 #include "DataFormats/Provenance/interface/ProcessHistoryID.h"
 #include "GeneratorInterface/LHEInterface/plugins/LHEProvenanceHelper.h"
 #include "FWCore/Sources/interface/ProducerSourceFromFiles.h"
@@ -37,9 +37,8 @@ class LHESource : public edm::ProducerSourceFromFiles {
   ~LHESource() override;
 
 private:
+
   void endJob() override;
-  void beginRun(edm::Run &run) override;
-  void endRun(edm::Run &run) override;
   bool setRunAndEventInfo(edm::EventID&, edm::TimeValue_t&, edm::EventAuxiliary::ExperimentType&) override;
   void readRun_(edm::RunPrincipal& runPrincipal) override;
   void readLuminosityBlock_(edm::LuminosityBlockPrincipal& lumiPrincipal) override;
@@ -50,17 +49,17 @@ class LHESource : public edm::ProducerSourceFromFiles {
 
   void nextEvent();
 
-  std::auto_ptr<lhef::LHEReader>		reader;
+  void putRunInfoProduct(edm::RunPrincipal&);
+  void fillRunInfoProduct(lhef::LHERunInfo const&, LHERunInfoProduct& );
+
+  std::unique_ptr<lhef::LHEReader>      reader_;
 
-  boost::shared_ptr<lhef::LHERunInfo>	runInfoLast;
-  boost::shared_ptr<lhef::LHERunInfo>	runInfo;
-  boost::shared_ptr<lhef::LHEEvent>	partonLevel;
+  boost::shared_ptr<lhef::LHERunInfo>	runInfoLast_;
+  boost::shared_ptr<lhef::LHEEvent>	partonLevel_;
 
-  boost::ptr_deque<LHERunInfoProduct>	runInfoProducts;
-  bool					wasMerged;
+  std::unique_ptr<LHERunInfoProduct>	runInfoProductLast_;
   edm::LHEProvenanceHelper		lheProvenanceHelper_;
   edm::ProcessHistoryID			phid_;
-  edm::RunPrincipal*	                runPrincipal_;
 };
 
 #endif // GeneratorInterface_LHEInterface_LHESource_h

GeneratorInterface/LHEInterface/src/LHEReader.cc

@@ -114,7 +114,12 @@ class LHEReader::XMLHandler : public XMLDocument::Handler {
 		kEvent
 	};
 
-        void reset() { headerOk = false; weightsinevent.clear();}
+        void reset() { 
+          headerOk = false; 
+          weightsinevent.clear();
+          gotObject = kNone;
+          mode = kNone;
+        }
 
         const wgt_info& weightInfo() const {return weightsinevent;}
 
@@ -486,6 +491,11 @@ LHEReader::~LHEReader()
   {
     while(curDoc.get() || curIndex < fileURLs.size() || (fileURLs.empty() && strName != "" ) ) {
       if (!curDoc.get()) {
+        if(!platform) {
+          //If we read multiple files, the XercesPlatform must live longer than any one
+          // XMLDocument.
+          platform = XMLDocument::platformHandle();
+        }
         if ( !fileURLs.empty() ) {
           logFileAction("  Initiating request to open LHE file ", fileURLs[curIndex]);
           curSource.reset(new FileSource(fileURLs[curIndex]));

GeneratorInterface/LHEInterface/src/XMLUtils.h

@@ -43,6 +43,7 @@ class XMLDocument {
 
 	bool parse();
 
+        static std::shared_ptr<void> platformHandle() { return std::make_shared<XercesPlatform>(); }
     private:
 	class XercesPlatform {
 	    public:

GeneratorInterface/LHEInterface/test/BuildFile.xml

@@ -4,3 +4,7 @@
   <use   name="FWCore/Framework"/>
   <flags   EDM_PLUGIN="1"/>
 </library>
+<bin   name="TestGeneratorInterfaceLHEInterfaceFileReading" file="TestDriver.cpp">
+  <flags   TEST_RUNNER_ARGS=" /bin/bash GeneratorInterface/LHEInterface/test testMerging.sh"/>
+  <use   name="FWCore/Utilities"/>
+</bin>

GeneratorInterface/LHEInterface/test/DummyLHEAnalyzer.cc

@@ -19,8 +19,10 @@ class DummyLHEAnalyzer : public EDAnalyzer {
 private: 
   bool dumpLHE_;
   bool checkPDG_;
+  bool dumpHeader_;
 public:
-  explicit DummyLHEAnalyzer( const ParameterSet & cfg ) : 
+  explicit DummyLHEAnalyzer( const ParameterSet & cfg ) :
+    dumpHeader_( cfg.getUntrackedParameter<bool>("dumpHeader",false) ),
     src_( cfg.getParameter<InputTag>( "src" ) ),
     tokenLHERunInfo_(consumes<LHERunInfoProduct,edm::InRun>(cfg.getUntrackedParameter<edm::InputTag>("moduleLabel", std::string("source")) ) ),
     tokenLHEEvent_(consumes<LHEEventProduct>(cfg.getUntrackedParameter<edm::InputTag>("moduleLabel", std::string("source")) ) )
@@ -105,6 +107,16 @@ class DummyLHEAnalyzer : public EDAnalyzer {
     }
     std::cout << " " << std::endl;
 
+    if(dumpHeader_) {
+      std::cout <<" HEADER "<<std::endl;
+      for(auto it = run->headers_begin(); it != run->headers_end(); ++it) {
+        std::cout <<"tag: '"<<it->tag()<<"'"<<std::endl;
+        for(auto const& l : it->lines()) {
+          std::cout<<"   "<<l<<std::endl;
+        }
+      }
+    }
+
   }
 
   InputTag src_;

GeneratorInterface/LHEInterface/test/TestDriver.cpp

@@ -0,0 +1,3 @@
+#include "FWCore/Utilities/interface/TestHelper.h"
+
+RUNTEST()