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