Changed set to vector in PFDisplacedVertexSeed

DataFormats/ParticleFlowReco/interface/PFDisplacedVertexSeed.h

@@ -4,7 +4,7 @@
 #include "DataFormats/GeometryVector/interface/GlobalPoint.h"
 #include "DataFormats/TrackReco/interface/TrackFwd.h"
 
-#include <set>
+#include <vector>
 #include <iostream>
 
 
@@ -29,26 +29,16 @@ namespace reco {
 
   public:
 
-    /// -------- Useful Types -------- ///
-
-    typedef std::set< reco::TrackBaseRef >::iterator IEset;
-
-    /// A function necessary to use a set format to store the tracks Refs.
-    /// The set is the most appropriate in that case to avoid the double counting 
-    /// of the tracks durring the build up procedure.
-    /// The position of the tracks in the Collection
-    /// is used as a classification parameter.
-    struct Compare{
-      bool operator()(const TrackBaseRef& s1, const TrackBaseRef& s2) const
-      {return s1.key() < s2.key();}
-    };
-
     /// Default constructor
     PFDisplacedVertexSeed();
 
     /// Add a track Reference to the current Seed
+    /// If the track reference is already in the collection, it is ignored
     void addElement(TrackBaseRef);
 
+    /// Reserve space for elements
+    void reserveElements(size_t);
+
     /// Add a track Ref to the Seed and recalculate the seedPoint with a new dcaPoint
     /// A weight different from 1 may be assign to the new DCA point 
     void updateSeedPoint(const GlobalPoint& dcaPoint, const TrackBaseRef, 
@@ -60,8 +50,8 @@ namespace reco {
     /// Check if it is a new Seed
     bool isEmpty() const {return (elements_.empty());}
 
-    /// \return set of references to tracks 
-    const std::set < TrackBaseRef, Compare >& elements() const 
+    /// \return vector of unique references to tracks 
+    const std::vector <TrackBaseRef>& elements() const 
       {return elements_;}
 
     const double nTracks() const {return elements_.size();}
@@ -84,7 +74,7 @@ namespace reco {
     /// --------- MEMBERS ---------- ///
 
     /// Set of tracks refs associated to the seed
-    std::set < TrackBaseRef , Compare >     elements_;
+    std::vector< TrackBaseRef>     elements_;
     /// Seed point which indicated the approximative position of the vertex.
     GlobalPoint                   seedPoint_;
     /// Total weight of the points used to calculate the seed point. 

DataFormats/ParticleFlowReco/src/PFDisplacedVertexSeed.cc

@@ -14,7 +14,13 @@ PFDisplacedVertexSeed::PFDisplacedVertexSeed() :
 
 
 void PFDisplacedVertexSeed::addElement(TrackBaseRef element) {
-  elements_.insert( element ); 
+  if(std::find(elements_.begin(),elements_.end(), element) == elements_.end()) {
+    elements_.emplace_back(std::move(element));
+  }
+}
+
+void PFDisplacedVertexSeed::reserveElements(size_t newSize) {
+  elements_.reserve(newSize);
 }
 
 
@@ -37,6 +43,7 @@ void PFDisplacedVertexSeed::updateSeedPoint(const GlobalPoint& dcaPoint, TrackBa
 
   }
 
+  reserveElements(elements_.size()+2);
   addElement(r1); 
   addElement(r2);
 
@@ -47,7 +54,6 @@ void PFDisplacedVertexSeed::mergeWith(const PFDisplacedVertexSeed& displacedVert
 
 
   double weight = displacedVertex.totalWeight();
-  const set<TrackBaseRef, Compare>& newElements= displacedVertex.elements();
   const GlobalPoint& dcaPoint = displacedVertex.seedPoint();
 
   Basic3DVector<double>vertexSeedVector(seedPoint_);
@@ -58,13 +64,14 @@ void PFDisplacedVertexSeed::mergeWith(const PFDisplacedVertexSeed& displacedVert
   totalWeight_ += weight;
   seedPoint_ = P;
 
-
-
-  for (  set<TrackBaseRef, Compare>::const_iterator il = newElements.begin(); il != newElements.end(); il++)
-    addElement(*il);
-
-
-
+  reserveElements(elements_.size()+displacedVertex.elements().size());
+  auto const oldSize=elements_.size();
+  //avoid checking elements we just added from displacedVertex.elements()
+  for(auto const& e: displacedVertex.elements()) {
+    if(std::find(elements_.begin(), elements_.begin()+oldSize,e) == elements_.begin()+oldSize) {
+      elements_.emplace_back(e);
+    }
+  }
 }
 
 
@@ -80,25 +87,25 @@ void PFDisplacedVertexSeed::Dump( ostream& out ) const {
 
   // Build element label (string) : elid from type, layer and occurence number
   // use stringstream instead of sprintf to concatenate string and integer into string
-  for(IEset ie = elements_.begin(); ie !=  elements_.end(); ie++){
+  for(auto const& ie : elements_) {
 
-    math::XYZPoint Pi((*ie).get()->innerPosition());
-    math::XYZPoint Po((*ie).get()->outerPosition());
+    math::XYZPoint Pi(ie.get()->innerPosition());
+    math::XYZPoint Po(ie.get()->outerPosition());
 
     float innermost_radius = sqrt(Pi.x()*Pi.x() + Pi.y()*Pi.y() + Pi.z()*Pi.z());
     float outermost_radius = sqrt(Po.x()*Po.x() + Po.y()*Po.y() + Po.z()*Po.z());
     float innermost_rho = sqrt(Pi.x()*Pi.x() + Pi.y()*Pi.y());
     float outermost_rho = sqrt(Po.x()*Po.x() + Po.y()*Po.y());
 
-    double pt = (*ie)->pt();
+    double pt = ie->pt();
 
 
-    out<<"ie = " << (*ie).key() << " pt = " << pt
+    out<<"ie = " << ie.key() << " pt = " << pt
        <<" innermost hit radius = " << innermost_radius << " rho = " << innermost_rho
        <<" outermost hit radius = " << outermost_radius << " rho = " << outermost_rho
        <<endl;
 
-    out<<"ie = " << (*ie).key() << " pt = " << pt
+    out<<"ie = " << ie.key() << " pt = " << pt
       //       <<" inn hit pos x = " << Pi.x() << " y = " << Pi.y() << " z = " << Pi.z() 
        <<" out hit pos x = " << Po.x() << " y = " << Po.y() << " z = " << Po.z() 
        <<endl;

DataFormats/ParticleFlowReco/test/BuildFile.xml

@@ -0,0 +1,6 @@
+<environment>
+  <bin file="test_catch2_*.cc" name="TestDataFormatsParticleFlowRecoCatch">
+    <use name="DataFormats/ParticleFlowReco"/>
+    <use name="catch2"/>
+  </bin>
+</environment>

DataFormats/ParticleFlowReco/test/test_catch2_PFDisplacedVertexSeed.cc

@@ -0,0 +1,104 @@
+#include "DataFormats/TrackReco/interface/Track.h"
+#include "DataFormats/ParticleFlowReco/interface/PFDisplacedVertexSeed.h"
+
+#include "catch.hpp"
+
+static constexpr auto s_tag = "[PFDisplacedVertexSeed]";
+TEST_CASE("Check adding elements", s_tag) {
+  reco::PFDisplacedVertexSeed seed;
+
+  REQUIRE(seed.elements().empty());
+
+  SECTION("updateSeedPoint") {
+
+    //empty tracks are fine
+    std::vector<reco::Track> tracks(5);
+
+    seed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,0)),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,1)) );
+    REQUIRE(seed.elements().size() == 2);
+    REQUIRE(seed.nTracks() == 2);
+
+    seed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,0)),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,1)) );
+    REQUIRE(seed.elements().size() == 2);
+    REQUIRE(seed.nTracks() == 2);
+
+    seed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,0)),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,2)) );
+    REQUIRE(seed.elements().size() == 3);
+    REQUIRE(seed.nTracks() == 3);
+
+    seed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,3)),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,4)) );
+    REQUIRE(seed.elements().size() == 5);
+    REQUIRE(seed.nTracks() == 5);
+
+  }
+
+  SECTION("addElement") {
+    //empty tracks are fine
+    std::vector<reco::Track> tracks(3);
+
+    seed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,0)),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,1)) );
+    REQUIRE(seed.elements().size() == 2);
+    REQUIRE(seed.nTracks() == 2);
+
+    seed.addElement(reco::TrackBaseRef(reco::TrackRef(&tracks,0)));
+    REQUIRE(seed.elements().size() == 2);
+    REQUIRE(seed.nTracks() == 2);
+
+    seed.addElement(reco::TrackBaseRef(reco::TrackRef(&tracks,2)));
+    REQUIRE(seed.elements().size() == 3);
+    REQUIRE(seed.nTracks() == 3);
+
+  }
+  SECTION("mergeWith") {
+    std::vector<reco::Track> tracks(5);
+
+    seed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,0)),
+                         reco::TrackBaseRef(reco::TrackRef(&tracks,1)) );
+
+
+    SECTION("completely overlapping seeds") {
+      reco::PFDisplacedVertexSeed otherSeed;
+      otherSeed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                                       reco::TrackBaseRef(reco::TrackRef(&tracks,0)),
+                                       reco::TrackBaseRef(reco::TrackRef(&tracks,1)) );
+
+      seed.mergeWith(otherSeed);
+      REQUIRE(seed.elements().size() == 2);
+    }
+
+    SECTION("partially overlapping seeds") {
+      reco::PFDisplacedVertexSeed otherSeed;
+      otherSeed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                                       reco::TrackBaseRef(reco::TrackRef(&tracks,0)),
+                                       reco::TrackBaseRef(reco::TrackRef(&tracks,2)) );
+
+      seed.mergeWith(otherSeed);
+      REQUIRE(seed.elements().size() == 3);
+    }
+
+    SECTION("non overlapping seeds") {
+      REQUIRE(seed.elements().size()==2);
+      reco::PFDisplacedVertexSeed otherSeed;
+      otherSeed.updateSeedPoint(GlobalPoint(0.01,0.01,0.01),
+                                       reco::TrackBaseRef(reco::TrackRef(&tracks,3)),
+                                       reco::TrackBaseRef(reco::TrackRef(&tracks,2)) );
+      REQUIRE(otherSeed.elements().size() == 2);
+
+      seed.mergeWith(otherSeed);
+      REQUIRE(seed.elements().size() == 4);
+    }
+
+  }
+}
+

DataFormats/ParticleFlowReco/test/test_catch2_main.cc

@@ -0,0 +1,2 @@
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"

RecoParticleFlow/PFTracking/src/PFDisplacedVertexFinder.cc

@@ -234,18 +234,16 @@ PFDisplacedVertexFinder::fitVertexFromSeed(const PFDisplacedVertexSeed& displace
 
   // ---- Prepare transient track list ----
 
-  set < TrackBaseRef, PFDisplacedVertexSeed::Compare > const& tracksToFit = displacedVertexSeed.elements();
+  auto const& tracksToFit = displacedVertexSeed.elements();
   const GlobalPoint& seedPoint = displacedVertexSeed.seedPoint();
 
   vector<TransientTrack> transTracks;
   vector<TransientTrack> transTracksRaw;
   vector<TrackBaseRef> transTracksRef;
-  vector<TrackBaseRef> transTracksRefRaw;
 
   transTracks.reserve(tracksToFit.size());
   transTracksRaw.reserve(tracksToFit.size());
   transTracksRef.reserve(tracksToFit.size());
-  transTracksRefRaw.reserve(tracksToFit.size());
 
 
 
@@ -278,7 +276,6 @@ PFDisplacedVertexFinder::fitVertexFromSeed(const PFDisplacedVertexSeed& displace
   for(auto const& ie : tracksToFit){
     TransientTrack tmpTk( *(ie.get()), magField_, globTkGeomHandle_);
     transTracksRaw.emplace_back( tmpTk );
-    transTracksRefRaw.push_back( ie );
     bool nonIt = PFTrackAlgoTools::nonIterative((ie)->algo());
     bool step45 = PFTrackAlgoTools::step45((ie)->algo());
     bool highQ = PFTrackAlgoTools::highQuality((ie)->algo());   
@@ -416,7 +413,7 @@ PFDisplacedVertexFinder::fitVertexFromSeed(const PFDisplacedVertexSeed& displace
 
     if (theVertexAdaptiveRaw.trackWeight(transTracksRaw[i]) > minAdaptWeight_){
 
-      PFTrackHitFullInfo pattern = hitPattern_.analyze(tkerTopo_, tkerGeom_, transTracksRefRaw[i], theVertexAdaptiveRaw);
+      PFTrackHitFullInfo pattern = hitPattern_.analyze(tkerTopo_, tkerGeom_, tracksToFit[i], theVertexAdaptiveRaw);
 
       PFDisplacedVertex::VertexTrackType vertexTrackType = getVertexTrackType(pattern);
 
@@ -426,7 +423,7 @@ PFDisplacedVertexFinder::fitVertexFromSeed(const PFDisplacedVertexSeed& displace
 
 	if (bGoodTrack){
 	  transTracks.push_back(transTracksRaw[i]);
-	  transTracksRef.push_back(transTracksRefRaw[i]);
+	  transTracksRef.push_back(tracksToFit[i]);
 	} else {
 	  if (debug_)
 	    cout << "Track rejected nChi2 = " << transTracksRaw[i].track().normalizedChi2()
@@ -528,7 +525,7 @@ PFDisplacedVertexFinder::fitVertexFromSeed(const PFDisplacedVertexSeed& displace
   // -----------------------------------------------//
 
 
-  displacedVertex = (PFDisplacedVertex) theRecoVtx;
+  displacedVertex = theRecoVtx;
   displacedVertex.removeTracks();
 
   for(unsigned i = 0; i < transTracks.size();i++) {

RecoParticleFlow/PFTracking/test/BuildFile.xml

@@ -7,3 +7,7 @@
 <library   file="PreIdAnalyzer.cc" name="PreIdAnalyzer">
   <flags   EDM_PLUGIN="1"/>
 </library>
+  <bin file="test_catch2_*.cc" name="TestRecoParticleFlowPFTrackingTP">
+    <use name="FWCore/TestProcessor"/>
+    <use name="catch2"/>
+  </bin>

RecoParticleFlow/PFTracking/test/test_catch2_PFDisplacedVertexProducer.cc

@@ -0,0 +1,67 @@
+#include "catch.hpp"
+#include "FWCore/TestProcessor/interface/TestProcessor.h"
+#include "FWCore/Utilities/interface/Exception.h"
+#include "DataFormats/ParticleFlowReco/interface/PFDisplacedVertexFwd.h"
+
+
+static constexpr auto s_tag = "[PFDisplacedVertexProducer]";
+
+TEST_CASE("Standard checks of PFDisplacedVertexProducer", s_tag) {
+  const std::string baseConfig{
+R"_(from FWCore.TestProcessor.TestProcess import *
+process = TestProcess()
+from RecoParticleFlow.PFTracking.particleFlowDisplacedVertex_cfi import particleFlowDisplacedVertex
+process.toTest = particleFlowDisplacedVertex
+process.moduleToTest(process.toTest)
+)_"
+  };
+
+  const std::string fullConfig{
+R"_(from FWCore.TestProcessor.TestProcess import *
+process = TestProcess()
+process.load("MagneticField.Engine.uniformMagneticField_cfi")
+process.load("Configuration.Geometry.GeometryExtended2018Reco_cff")
+process.add_(cms.ESProducer("TrackerParametersESModule"))
+process.load("Alignment.CommonAlignmentProducer.FakeAlignmentSource_cfi")
+
+
+from RecoParticleFlow.PFTracking.particleFlowDisplacedVertex_cfi import particleFlowDisplacedVertex
+process.toTest = particleFlowDisplacedVertex
+process.moduleToTest(process.toTest)
+)_"
+  };
+
+  edm::test::TestProcessor::Config config{ baseConfig };  
+  SECTION("base configuration is OK") {
+    REQUIRE_NOTHROW(edm::test::TestProcessor(config));
+  }
+
+  SECTION("No event data") {
+    edm::test::TestProcessor::Config config{ fullConfig };  
+    edm::test::TestProcessor tester(config);
+
+    //The module ignores missing data products
+    REQUIRE(tester.test().get<reco::PFDisplacedVertexCollection>()->empty());
+  }
+
+  SECTION("beginJob and endJob only") {
+    edm::test::TestProcessor tester(config);
+
+    REQUIRE_NOTHROW(tester.testBeginAndEndJobOnly());
+  }
+
+  SECTION("Run with no LuminosityBlocks") {
+    edm::test::TestProcessor tester(config);
+
+    REQUIRE_NOTHROW(tester.testRunWithNoLuminosityBlocks());
+  }
+
+  SECTION("LuminosityBlock with no Events") {
+    edm::test::TestProcessor tester(config);
+
+    REQUIRE_NOTHROW(tester.testLuminosityBlockWithNoEvents());
+  }
+
+}
+
+//Add additional TEST_CASEs to exercise the modules capabilities

RecoParticleFlow/PFTracking/test/test_catch2_main.cc

@@ -0,0 +1,2 @@
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"