From bbc479e1d90a3be41c893015f17fe5e829447b0a Mon Sep 17 00:00:00 2001
From: Chris Jones <jonesc@hep.phy.cam.ac.uk>
Date: Wed, 17 Aug 2016 11:19:52 +0100
Subject: [PATCH 1/4] Add checks to MCParticle and MCVertex unpackers to filter
 out duplicates.

---
 Event/EventPacker/doc/release.notes               |  3 +++
 .../EventPacker/src/component/PackMCParticle.cpp  | 12 +++++++-----
 Event/EventPacker/src/component/PackMCVertex.cpp  |  9 ++++-----
 .../src/component/UnpackMCParticle.cpp            | 13 +++++++++++--
 .../EventPacker/src/component/UnpackMCVertex.cpp  | 15 +++++++++++++--
 5 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/Event/EventPacker/doc/release.notes b/Event/EventPacker/doc/release.notes
index 47ce900eeab..427733ba00f 100644
--- a/Event/EventPacker/doc/release.notes
+++ b/Event/EventPacker/doc/release.notes
@@ -4,6 +4,9 @@
 ! Purpose     : Pack and unpack events classes for smaller events
 !-----------------------------------------------------------------------------
 
+! 2016-08-17 - Chris Jones
+ - Add checks to MCParticle and MCVertex unpackers to filter out duplicates.
+
 ! 2016-07-15 - Chris Jones
  - Improve error messages from PackCluster.
  - Make cluster locations in PackCluster configurable options.
diff --git a/Event/EventPacker/src/component/PackMCParticle.cpp b/Event/EventPacker/src/component/PackMCParticle.cpp
index a11a7cfe996..a5845105a44 100755
--- a/Event/EventPacker/src/component/PackMCParticle.cpp
+++ b/Event/EventPacker/src/component/PackMCParticle.cpp
@@ -1,4 +1,7 @@
 
+// STL
+#include <algorithm>
+
 #include "Event/MCParticle.h"
 #include "Event/StandardPacker.h"
 #include "Event/PackedMCParticle.h"
@@ -69,7 +72,7 @@ StatusCode PackMCParticle::execute() {
   for ( const LHCb::MCParticle * part : *parts )
   {
     out->mcParts().emplace_back( LHCb::PackedMCParticle() );
-    LHCb::PackedMCParticle& newPart = out->mcParts().back();
+    auto & newPart = out->mcParts().back();
 
     newPart.key   = part->key();
     newPart.px    = pack.energy( part->momentum().px() );
@@ -85,12 +88,11 @@ StatusCode PackMCParticle::execute() {
                              pack.reference64( out,
                                                part->originVertex()->parent(),
                                                part->originVertex()->key() ) );
-    for ( SmartRefVector<LHCb::MCVertex>::const_iterator itV = part->endVertices().begin();
-          part->endVertices().end() != itV; ++itV ) 
+    for ( const auto & V : part->endVertices() )
     {
       newPart.endVertices.push_back( 0==pVer ? 
-                                     pack.reference32( out, (*itV)->parent(), (*itV)->key() ) :
-                                     pack.reference64( out, (*itV)->parent(), (*itV)->key() ) );
+                                     pack.reference32( out, V->parent(), V->key() ) :
+                                     pack.reference64( out, V->parent(), V->key() ) );
     }
 
   }
diff --git a/Event/EventPacker/src/component/PackMCVertex.cpp b/Event/EventPacker/src/component/PackMCVertex.cpp
index 59035513fc9..21507578c36 100755
--- a/Event/EventPacker/src/component/PackMCVertex.cpp
+++ b/Event/EventPacker/src/component/PackMCVertex.cpp
@@ -77,7 +77,7 @@ StatusCode PackMCVertex::execute()
   for ( const LHCb::MCVertex* vert : *verts )
   {
     out->mcVerts().emplace_back( LHCb::PackedMCVertex() );
-    LHCb::PackedMCVertex& newVert = out->mcVerts().back();
+    auto & newVert = out->mcVerts().back();
 
     newVert.key  = vert->key();
     newVert.x    = pack.position( vert->position().x() );
@@ -125,12 +125,11 @@ StatusCode PackMCVertex::execute()
                          pack.reference64( out, vert->mother()->parent(),
                                            vert->mother()->key() ) );
     }
-    for ( SmartRefVector<LHCb::MCParticle>::const_iterator itP = vert->products().begin();
-          vert->products().end() != itP; ++itP ) 
+    for ( const auto & P : vert->products() )
     {
       newVert.products.push_back( 0==pVer ? 
-                                  pack.reference32( out, (*itP)->parent(), (*itP)->key() ) :
-                                  pack.reference64( out, (*itP)->parent(), (*itP)->key() ) );
+                                  pack.reference32( out, P->parent(), P->key() ) :
+                                  pack.reference64( out, P->parent(), P->key() ) );
     }
 
     if( msgLevel(MSG::VERBOSE) ) verbose() << "Vertex packed OK" << endmsg;
diff --git a/Event/EventPacker/src/component/UnpackMCParticle.cpp b/Event/EventPacker/src/component/UnpackMCParticle.cpp
index 25d53ba2e74..3ab232fc9b2 100755
--- a/Event/EventPacker/src/component/UnpackMCParticle.cpp
+++ b/Event/EventPacker/src/component/UnpackMCParticle.cpp
@@ -1,6 +1,7 @@
 
 // STL
 //#include <random> // For flags tests. To be removed.
+#include <algorithm>
 
 // Event model
 #include "Event/MCParticle.h"
@@ -99,8 +100,16 @@ StatusCode UnpackMCParticle::execute()
       if ( ( 0==pVer && pack.hintAndKey32( I, dst, newMCParticles, hintID, key ) ) ||
            ( 0!=pVer && pack.hintAndKey64( I, dst, newMCParticles, hintID, key ) ) )
       {
-        SmartRef<LHCb::MCVertex> refV( newMCParticles, hintID, key );
-        part->addToEndVertices( refV );
+        // Construct the smart ref
+        SmartRef<LHCb::MCVertex> ref( newMCParticles, hintID, key );
+        // Do we already have a reference to this vertex ?
+        const bool found = std::any_of( part->endVertices().begin(),
+                                        part->endVertices().end(),
+                                        [&ref]( const auto& sref )
+                                        { return sref.target() == ref.target(); } );
+        if ( !found ) { part->addToEndVertices( ref ); }
+        else { Warning( "Found duplicate in packed MCParticle end vertices", 
+                        StatusCode::SUCCESS ).ignore(); }
       }
       else { Error( "Corrupt MCParticle End MCVertex SmartRef detected" ).ignore(); }
     }
diff --git a/Event/EventPacker/src/component/UnpackMCVertex.cpp b/Event/EventPacker/src/component/UnpackMCVertex.cpp
index d8b8ee06dd6..f92cf80b587 100755
--- a/Event/EventPacker/src/component/UnpackMCVertex.cpp
+++ b/Event/EventPacker/src/component/UnpackMCVertex.cpp
@@ -1,6 +1,8 @@
 // $Id: UnpackMCVertex.cpp,v 1.5 2009-11-07 12:20:39 jonrob Exp $
 // Include files 
 
+#include <algorithm>
+
 #include "Event/MCVertex.h"
 #include "Event/StandardPacker.h"
 #include "Event/PackedMCVertex.h"
@@ -80,14 +82,23 @@ StatusCode UnpackMCVertex::execute()
       }
       else { Error( "Corrupt MCVertex Mother MCParticle SmartRef detected" ).ignore(); }
     }
-    
+
+    // Check for duplicates ...
     for ( const auto& I : src.products )
     {
       if ( ( 0==pVer && pack.hintAndKey32( I, dst, newMCVertices, hintID, key ) ) ||
            ( 0!=pVer && pack.hintAndKey64( I, dst, newMCVertices, hintID, key ) ) )
       {
+        // Construct the smart ref
         SmartRef<LHCb::MCParticle> ref( newMCVertices, hintID, key );
-        vert->addToProducts( ref );
+        // Do we already have a reference to this particle ?
+        const bool found = std::any_of( vert->products().begin(),
+                                        vert->products().end(),
+                                        [&ref]( const auto& sref )
+                                        { return sref.target() == ref.target(); } );
+        if ( !found ) { vert->addToProducts( ref ); }
+        else { Warning( "Found duplication in packed MCVertex products.",
+                        StatusCode::SUCCESS ).ignore(); }
       }
       else { Error( "Corrupt MCVertex Daughter MCParticle SmartRef detected" ).ignore(); }
     }
-- 
GitLab


From 22c1b827e24fe1ea51f12c1d82436bee15293ad8 Mon Sep 17 00:00:00 2001
From: Chris Jones <jonesc@hep.phy.cam.ac.uk>
Date: Thu, 18 Aug 2016 10:35:32 +0100
Subject: [PATCH 2/4] Rework duplicate checks to avoid SmartRef dereferencing

---
 .../src/component/UnpackMCParticle.cpp        | 54 +++++++++++--------
 .../src/component/UnpackMCVertex.cpp          | 45 ++++++++++------
 2 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/Event/EventPacker/src/component/UnpackMCParticle.cpp b/Event/EventPacker/src/component/UnpackMCParticle.cpp
index 3ab232fc9b2..65b9ddf8aa8 100755
--- a/Event/EventPacker/src/component/UnpackMCParticle.cpp
+++ b/Event/EventPacker/src/component/UnpackMCParticle.cpp
@@ -67,17 +67,17 @@ StatusCode UnpackMCParticle::execute()
   //static std::uniform_real_distribution<float> uniform(0,1);
 
   newMCParticles->reserve( dst->mcParts().size() );
-  for ( const LHCb::PackedMCParticle& src : dst->mcParts() )
+  for ( auto & src : dst->mcParts() )
   {
 
-    LHCb::MCParticle* part = new LHCb::MCParticle( );
+    LHCb::MCParticle * part = new LHCb::MCParticle( );
     newMCParticles->insert( part, src.key );
 
-    const double px = pack.energy( src.px );
-    const double py = pack.energy( src.py );
-    const double pz = pack.energy( src.pz );
-    const double mass = src.mass;
-    const double E = std::sqrt( (px*px) + (py*py) + (pz*pz) + (mass*mass) );
+    const auto px = pack.energy( src.px );
+    const auto py = pack.energy( src.py );
+    const auto pz = pack.energy( src.pz );
+    const auto mass = src.mass;
+    const auto E = std::sqrt( (px*px) + (py*py) + (pz*pz) + (mass*mass) );
     part->setMomentum( Gaudi::LorentzVector( px, py, pz , E ) );
 
     part->setParticleID( LHCb::ParticleID(src.PID) );
@@ -95,26 +95,38 @@ StatusCode UnpackMCParticle::execute()
     }
     else { Error( "Corrupt MCParticle Origin MCVertex SmartRef detected" ).ignore(); }
 
+    // List of processed refs, to check for duplicates
+    std::vector<long long> processedRefs;
+    processedRefs.reserve( src.endVertices.size() );
+
+    // loop over refs and process
     for ( const auto& I : src.endVertices )
     {
-      if ( ( 0==pVer && pack.hintAndKey32( I, dst, newMCParticles, hintID, key ) ) ||
-           ( 0!=pVer && pack.hintAndKey64( I, dst, newMCParticles, hintID, key ) ) )
+      // Check for duplicates ...
+      if ( std::find( processedRefs.begin(), 
+                      processedRefs.end(), I ) == processedRefs.end() )
       {
-        // Construct the smart ref
-        SmartRef<LHCb::MCVertex> ref( newMCParticles, hintID, key );
-        // Do we already have a reference to this vertex ?
-        const bool found = std::any_of( part->endVertices().begin(),
-                                        part->endVertices().end(),
-                                        [&ref]( const auto& sref )
-                                        { return sref.target() == ref.target(); } );
-        if ( !found ) { part->addToEndVertices( ref ); }
-        else { Warning( "Found duplicate in packed MCParticle end vertices", 
-                        StatusCode::SUCCESS ).ignore(); }
+        // save this packed ref to the list of those already processed.
+        processedRefs.push_back(I);
+        // Unpack the ref and save to the vertex
+        hintID = key = 0;
+        if ( ( 0==pVer && pack.hintAndKey32( I, dst, newMCParticles, hintID, key ) ) ||
+             ( 0!=pVer && pack.hintAndKey64( I, dst, newMCParticles, hintID, key ) ) )
+        {
+          // Construct the smart ref
+          SmartRef<LHCb::MCVertex> ref( newMCParticles, hintID, key );
+          // save
+          part->addToEndVertices( ref );
+        }
+        else { Error( "Corrupt MCParticle End MCVertex SmartRef detected" ).ignore(); }
       }
-      else { Error( "Corrupt MCParticle End MCVertex SmartRef detected" ).ignore(); }
+      else
+      {
+        Warning( "Found duplicate in packed MCParticle end vertices", 
+                 StatusCode::SUCCESS ).ignore(); }
     }
   }
-
+  
   return StatusCode::SUCCESS;
 }
 
diff --git a/Event/EventPacker/src/component/UnpackMCVertex.cpp b/Event/EventPacker/src/component/UnpackMCVertex.cpp
index f92cf80b587..3b615521c90 100755
--- a/Event/EventPacker/src/component/UnpackMCVertex.cpp
+++ b/Event/EventPacker/src/component/UnpackMCVertex.cpp
@@ -59,7 +59,7 @@ StatusCode UnpackMCVertex::execute()
   const char pVer = dst->packingVersion();
 
   newMCVertices->reserve( dst->mcVerts().size() );
-  for ( const LHCb::PackedMCVertex& src : dst->mcVerts() )
+  for ( auto & src : dst->mcVerts() )
   {
 
     auto* vert = new LHCb::MCVertex( );
@@ -70,8 +70,7 @@ StatusCode UnpackMCVertex::execute()
     vert->setTime( src.tof );
     vert->setType( (LHCb::MCVertex::MCVertexType) src.type );
 
-    int hintID;
-    int key;
+    int hintID(0), key(0);
     if ( -1 != src.mother )
     {
       if ( ( 0==pVer && pack.hintAndKey32( src.mother, dst, newMCVertices, hintID, key ) ) ||
@@ -83,24 +82,36 @@ StatusCode UnpackMCVertex::execute()
       else { Error( "Corrupt MCVertex Mother MCParticle SmartRef detected" ).ignore(); }
     }
 
-    // Check for duplicates ...
+    // List of processed refs, to check for duplicates
+    std::vector<long long> processedRefs;
+    processedRefs.reserve( src.products.size() );
+
+    // loop over refs and process
     for ( const auto& I : src.products )
     {
-      if ( ( 0==pVer && pack.hintAndKey32( I, dst, newMCVertices, hintID, key ) ) ||
-           ( 0!=pVer && pack.hintAndKey64( I, dst, newMCVertices, hintID, key ) ) )
+      // Check for duplicates ...
+      if ( std::find( processedRefs.begin(), 
+                      processedRefs.end(), I ) == processedRefs.end() )
       {
-        // Construct the smart ref
-        SmartRef<LHCb::MCParticle> ref( newMCVertices, hintID, key );
-        // Do we already have a reference to this particle ?
-        const bool found = std::any_of( vert->products().begin(),
-                                        vert->products().end(),
-                                        [&ref]( const auto& sref )
-                                        { return sref.target() == ref.target(); } );
-        if ( !found ) { vert->addToProducts( ref ); }
-        else { Warning( "Found duplication in packed MCVertex products.",
-                        StatusCode::SUCCESS ).ignore(); }
+        // save this packed ref to the list of those already processed.
+        processedRefs.push_back(I);
+        // Unpack the ref and save to the vertex
+        hintID = key = 0;
+        if ( ( 0==pVer && pack.hintAndKey32( I, dst, newMCVertices, hintID, key ) ) ||
+             ( 0!=pVer && pack.hintAndKey64( I, dst, newMCVertices, hintID, key ) ) )
+        {
+          // Construct the smart ref
+          SmartRef<LHCb::MCParticle> ref( newMCVertices, hintID, key );
+          // save
+          vert->addToProducts( ref );
+        }
+        else { Error( "Corrupt MCVertex Daughter MCParticle SmartRef detected" ).ignore(); }
+      }
+      else
+      {
+        Warning( "Found duplicate in packed MCVertex products", 
+                 StatusCode::SUCCESS ).ignore();
       }
-      else { Error( "Corrupt MCVertex Daughter MCParticle SmartRef detected" ).ignore(); }
     }
   }
   
-- 
GitLab


From 6440e2afae670e6f1a2adcb8e391af893b799908 Mon Sep 17 00:00:00 2001
From: Chris Jones <jonesc@hep.phy.cam.ac.uk>
Date: Thu, 18 Aug 2016 10:39:33 +0100
Subject: [PATCH 3/4] make const

---
 Event/EventPacker/src/component/UnpackMCParticle.cpp | 2 +-
 Event/EventPacker/src/component/UnpackMCVertex.cpp   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Event/EventPacker/src/component/UnpackMCParticle.cpp b/Event/EventPacker/src/component/UnpackMCParticle.cpp
index 65b9ddf8aa8..a27d5c412e2 100755
--- a/Event/EventPacker/src/component/UnpackMCParticle.cpp
+++ b/Event/EventPacker/src/component/UnpackMCParticle.cpp
@@ -67,7 +67,7 @@ StatusCode UnpackMCParticle::execute()
   //static std::uniform_real_distribution<float> uniform(0,1);
 
   newMCParticles->reserve( dst->mcParts().size() );
-  for ( auto & src : dst->mcParts() )
+  for ( const auto & src : dst->mcParts() )
   {
 
     LHCb::MCParticle * part = new LHCb::MCParticle( );
diff --git a/Event/EventPacker/src/component/UnpackMCVertex.cpp b/Event/EventPacker/src/component/UnpackMCVertex.cpp
index 3b615521c90..b0e42b93382 100755
--- a/Event/EventPacker/src/component/UnpackMCVertex.cpp
+++ b/Event/EventPacker/src/component/UnpackMCVertex.cpp
@@ -59,7 +59,7 @@ StatusCode UnpackMCVertex::execute()
   const char pVer = dst->packingVersion();
 
   newMCVertices->reserve( dst->mcVerts().size() );
-  for ( auto & src : dst->mcVerts() )
+  for ( const auto & src : dst->mcVerts() )
   {
 
     auto* vert = new LHCb::MCVertex( );
-- 
GitLab


From a3f4ffb4e9dc8042f2ef0d4962460be8f0abb8e6 Mon Sep 17 00:00:00 2001
From: Chris Jones <jonesc@hep.phy.cam.ac.uk>
Date: Thu, 18 Aug 2016 10:49:22 +0100
Subject: [PATCH 4/4] use std::any_of

---
 Event/EventPacker/src/component/UnpackMCParticle.cpp | 4 ++--
 Event/EventPacker/src/component/UnpackMCVertex.cpp   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Event/EventPacker/src/component/UnpackMCParticle.cpp b/Event/EventPacker/src/component/UnpackMCParticle.cpp
index a27d5c412e2..23005c66637 100755
--- a/Event/EventPacker/src/component/UnpackMCParticle.cpp
+++ b/Event/EventPacker/src/component/UnpackMCParticle.cpp
@@ -103,8 +103,8 @@ StatusCode UnpackMCParticle::execute()
     for ( const auto& I : src.endVertices )
     {
       // Check for duplicates ...
-      if ( std::find( processedRefs.begin(), 
-                      processedRefs.end(), I ) == processedRefs.end() )
+      if ( !std::any_of( processedRefs.begin(), processedRefs.end(),
+                         [&I]( const auto J ) { return I == J; } ) )
       {
         // save this packed ref to the list of those already processed.
         processedRefs.push_back(I);
diff --git a/Event/EventPacker/src/component/UnpackMCVertex.cpp b/Event/EventPacker/src/component/UnpackMCVertex.cpp
index b0e42b93382..868e33011b9 100755
--- a/Event/EventPacker/src/component/UnpackMCVertex.cpp
+++ b/Event/EventPacker/src/component/UnpackMCVertex.cpp
@@ -90,8 +90,8 @@ StatusCode UnpackMCVertex::execute()
     for ( const auto& I : src.products )
     {
       // Check for duplicates ...
-      if ( std::find( processedRefs.begin(), 
-                      processedRefs.end(), I ) == processedRefs.end() )
+      if ( !std::any_of( processedRefs.begin(), processedRefs.end(),
+                         [&I]( const auto J ) { return I == J; } ) )
       {
         // save this packed ref to the list of those already processed.
         processedRefs.push_back(I);
-- 
GitLab