From d85d3aa84183275b41293ab4f0a0eaeb44ac1909 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Fri, 17 Nov 2023 15:25:31 -0600 Subject: [PATCH 1/7] General RefToBase to Ptr conversion --- DataFormats/Common/interface/RefToBaseToPtr.h | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 DataFormats/Common/interface/RefToBaseToPtr.h diff --git a/DataFormats/Common/interface/RefToBaseToPtr.h b/DataFormats/Common/interface/RefToBaseToPtr.h new file mode 100644 index 0000000000000..0f20f6e919131 --- /dev/null +++ b/DataFormats/Common/interface/RefToBaseToPtr.h @@ -0,0 +1,36 @@ +#ifndef DataFormats_Common_RefToBaseToPtr_h +#define DataFormats_Common_RefToBaseToPtr_h + +/*---------------------------------------------------------------------- + +Ref: A function template for conversion from RefToBase to Ptr + +----------------------------------------------------------------------*/ +/* + ----------------------------------------------------------------------*/ + +#include "DataFormats/Common/interface/RefToBase.h" +#include "DataFormats/Common/interface/Ptr.h" + +namespace edm { + template + Ptr refToBaseToPtr(RefToBase const& ref) { + if (ref.isNull()) { + return Ptr(); + } + if (ref.isTransient()) { + return Ptr(ref.get(), ref.key()); + } else { + //Another thread could change this value so get only once + EDProductGetter const* getter = ref.productGetter(); + if (getter) { + return Ptr(ref.id(), ref.key(), getter); + } + } + // If this is called in an iorule outside the framework, we cannot call + // ref.get() but since the Ptr will be able to get it later anyway, we can + // fill with a nullptr for now + return Ptr(ref.id(), static_cast(nullptr), ref.key()); + } +} // namespace edm +#endif From a3c8e965476f218a033bec634709a6c1fa94f7bb Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Mon, 20 Nov 2023 13:07:33 -0600 Subject: [PATCH 2/7] Separate RefToBase to Ptr implementation for ioread --- DataFormats/Common/interface/RefToBaseToPtr.h | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/DataFormats/Common/interface/RefToBaseToPtr.h b/DataFormats/Common/interface/RefToBaseToPtr.h index 0f20f6e919131..8e011c1764d8f 100644 --- a/DataFormats/Common/interface/RefToBaseToPtr.h +++ b/DataFormats/Common/interface/RefToBaseToPtr.h @@ -27,9 +27,32 @@ namespace edm { return Ptr(ref.id(), ref.key(), getter); } } + return Ptr(ref.id(), ref.get(), ref.key()); + } + + /* + * Do not use this ouside an ioread rule in classes_def.xml + */ + template + Ptr refToBaseToPtr_ioread(RefToBase const& ref) { + if (ref.isNull()) { + return Ptr(); + } + if (ref.isTransient()) { + // This is a logic error, any ref being deserialized + // by construction would be persistent + return Ptr(); + } else { + //Another thread could change this value so get only once + EDProductGetter const* getter = ref.productGetter(); + if (getter) { + return Ptr(ref.id(), ref.key(), getter); + } + } // If this is called in an iorule outside the framework, we cannot call - // ref.get() but since the Ptr will be able to get it later anyway, we can - // fill with a nullptr for now + // ref.get(), but since outside the framework we can never fetch the ref, + // the Ptr will only be useful if accessed later from inside the framework. + // We can fill with a nullptr for now return Ptr(ref.id(), static_cast(nullptr), ref.key()); } } // namespace edm From 95784756f04c44cc5dd4819b26ef6831615483bd Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Mon, 20 Nov 2023 14:55:46 -0600 Subject: [PATCH 3/7] Use general method to migrate TriggerObject.refToOrig_ member Deprecate any API methods that return RefToBase --- .../PatCandidates/interface/TriggerObject.h | 28 +++++----- .../PatCandidates/src/TriggerObject.cc | 56 +------------------ .../PatCandidates/src/classes_def_trigger.xml | 9 ++- 3 files changed, 24 insertions(+), 69 deletions(-) diff --git a/DataFormats/PatCandidates/interface/TriggerObject.h b/DataFormats/PatCandidates/interface/TriggerObject.h index 4ac711b365b6b..a089ae4a85eda 100644 --- a/DataFormats/PatCandidates/interface/TriggerObject.h +++ b/DataFormats/PatCandidates/interface/TriggerObject.h @@ -53,7 +53,7 @@ namespace pat { /// Reference to trigger object, /// meant for 'l1extra' particles to access their additional functionalities, /// empty otherwise - reco::CandidateBaseRef refToOrig_; + reco::CandidatePtr refToOrig_; public: /// Constructors and Destructor @@ -117,37 +117,39 @@ namespace pat { /// Special methods for 'l1extra' particles /// General getters - const reco::CandidateBaseRef& origObjRef() const { return refToOrig_; }; const reco::Candidate* origObjCand() const { return refToOrig_.get(); }; /// Getters specific to the 'l1extra' particle type for /// - EM - const l1extra::L1EmParticleRef origL1EmRef() const; const L1GctEmCand* origL1GctEmCand() const { - return origL1EmRef().isNonnull() ? origL1EmRef()->gctEmCand() : nullptr; + auto* cand = dynamic_cast(origObjCand()); + return cand ? cand->gctEmCand() : nullptr; }; /// - EtMiss - const l1extra::L1EtMissParticleRef origL1EtMissRef() const; const L1GctEtMiss* origL1GctEtMiss() const { - return origL1EtMissRef().isNonnull() ? origL1EtMissRef()->gctEtMiss() : nullptr; + auto* cand = dynamic_cast(origObjCand()); + return cand ? cand->gctEtMiss() : nullptr; }; const L1GctEtTotal* origL1GctEtTotal() const { - return origL1EtMissRef().isNonnull() ? origL1EtMissRef()->gctEtTotal() : nullptr; + auto* cand = dynamic_cast(origObjCand()); + return cand ? cand->gctEtTotal() : nullptr; }; const L1GctHtMiss* origL1GctHtMiss() const { - return origL1EtMissRef().isNonnull() ? origL1EtMissRef()->gctHtMiss() : nullptr; + auto* cand = dynamic_cast(origObjCand()); + return cand ? cand->gctHtMiss() : nullptr; }; const L1GctEtHad* origL1GctEtHad() const { - return origL1EtMissRef().isNonnull() ? origL1EtMissRef()->gctEtHad() : nullptr; + auto* cand = dynamic_cast(origObjCand()); + return cand ? cand->gctEtHad() : nullptr; }; /// - Jet - const l1extra::L1JetParticleRef origL1JetRef() const; const L1GctJetCand* origL1GctJetCand() const { - return origL1JetRef().isNonnull() ? origL1JetRef()->gctJetCand() : nullptr; + auto* cand = dynamic_cast(origObjCand()); + return cand ? cand->gctJetCand() : nullptr; }; /// - Muon - const l1extra::L1MuonParticleRef origL1MuonRef() const; const L1MuGMTExtendedCand* origL1GmtMuonCand() const { - return origL1MuonRef().isNonnull() ? &(origL1MuonRef()->gmtMuonCand()) : nullptr; + auto* cand = dynamic_cast(origObjCand()); + return cand ? &(cand->gmtMuonCand()) : nullptr; }; /// Special methods for the cut string parser diff --git a/DataFormats/PatCandidates/src/TriggerObject.cc b/DataFormats/PatCandidates/src/TriggerObject.cc index 66dc0ef2bdb72..d530467219238 100644 --- a/DataFormats/PatCandidates/src/TriggerObject.cc +++ b/DataFormats/PatCandidates/src/TriggerObject.cc @@ -3,6 +3,7 @@ #include "DataFormats/PatCandidates/interface/TriggerObject.h" +#include "DataFormats/Common/interface/RefToBaseToPtr.h" #include "FWCore/Utilities/interface/EDMException.h" using namespace pat; @@ -25,7 +26,7 @@ TriggerObject::TriggerObject(const reco::LeafCandidate& leafCand) : reco::LeafCa // Constructors from base candidate reference (for 'l1extra' particles) TriggerObject::TriggerObject(const reco::CandidateBaseRef& candRef) - : reco::LeafCandidate(*candRef), refToOrig_(candRef) { + : reco::LeafCandidate(*candRef), refToOrig_(edm::refToBaseToPtr(candRef)) { triggerObjectTypes_.clear(); } @@ -79,56 +80,3 @@ bool TriggerObject::hasTriggerObjectType(trigger::TriggerObjectType triggerObjec return false; } -// Special methods for 'l1extra' particles - -// Getters specific to the 'l1extra' particle types -// Exceptions of type 'edm::errors::InvalidReference' are thrown, -// if wrong particle type is requested - -// EM -const l1extra::L1EmParticleRef TriggerObject::origL1EmRef() const { - l1extra::L1EmParticleRef l1Ref; - try { - l1Ref = origObjRef().castTo(); - } catch (edm::Exception const& X) { - if (X.categoryCode() != edm::errors::InvalidReference) - throw X; - } - return l1Ref; -} - -// EtMiss -const l1extra::L1EtMissParticleRef TriggerObject::origL1EtMissRef() const { - l1extra::L1EtMissParticleRef l1Ref; - try { - l1Ref = origObjRef().castTo(); - } catch (edm::Exception const& X) { - if (X.categoryCode() != edm::errors::InvalidReference) - throw X; - } - return l1Ref; -} - -// Jet -const l1extra::L1JetParticleRef TriggerObject::origL1JetRef() const { - l1extra::L1JetParticleRef l1Ref; - try { - l1Ref = origObjRef().castTo(); - } catch (edm::Exception const& X) { - if (X.categoryCode() != edm::errors::InvalidReference) - throw X; - } - return l1Ref; -} - -// Muon -const l1extra::L1MuonParticleRef TriggerObject::origL1MuonRef() const { - l1extra::L1MuonParticleRef l1Ref; - try { - l1Ref = origObjRef().castTo(); - } catch (edm::Exception const& X) { - if (X.categoryCode() != edm::errors::InvalidReference) - throw X; - } - return l1Ref; -} diff --git a/DataFormats/PatCandidates/src/classes_def_trigger.xml b/DataFormats/PatCandidates/src/classes_def_trigger.xml index d6ccc0be3df01..6a2847284d6c0 100644 --- a/DataFormats/PatCandidates/src/classes_def_trigger.xml +++ b/DataFormats/PatCandidates/src/classes_def_trigger.xml @@ -1,11 +1,15 @@ - + + + + + @@ -26,7 +30,8 @@ - + + From b1b1be3fb58387f49a988bd9617b13ee2fec4f90 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Mon, 20 Nov 2023 16:52:02 -0600 Subject: [PATCH 4/7] Use a correct ioread rule --- DataFormats/PatCandidates/src/classes_def_trigger.xml | 4 ++-- DataFormats/PatCandidates/src/classes_trigger.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/DataFormats/PatCandidates/src/classes_def_trigger.xml b/DataFormats/PatCandidates/src/classes_def_trigger.xml index 6a2847284d6c0..011b32f1a8dee 100644 --- a/DataFormats/PatCandidates/src/classes_def_trigger.xml +++ b/DataFormats/PatCandidates/src/classes_def_trigger.xml @@ -7,8 +7,8 @@ - - + + diff --git a/DataFormats/PatCandidates/src/classes_trigger.h b/DataFormats/PatCandidates/src/classes_trigger.h index 28273cb73f27f..10c35dd742450 100644 --- a/DataFormats/PatCandidates/src/classes_trigger.h +++ b/DataFormats/PatCandidates/src/classes_trigger.h @@ -2,6 +2,7 @@ #include "DataFormats/Common/interface/Association.h" #include "DataFormats/Common/interface/Wrapper.h" #include "DataFormats/Common/interface/PtrVector.h" +#include "DataFormats/Common/interface/RefToBaseToPtr.h" #include "DataFormats/PatCandidates/interface/TriggerObjectStandAlone.h" #include "DataFormats/PatCandidates/interface/TriggerEvent.h" From 30130d39d211392910af9922b2eb2aeaaa46ae8b Mon Sep 17 00:00:00 2001 From: Nicholas Smith Date: Tue, 21 Nov 2023 14:39:40 -0600 Subject: [PATCH 5/7] Address comments on comments --- DataFormats/Common/interface/RefToBaseToPtr.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/DataFormats/Common/interface/RefToBaseToPtr.h b/DataFormats/Common/interface/RefToBaseToPtr.h index 8e011c1764d8f..22c6451cfaa12 100644 --- a/DataFormats/Common/interface/RefToBaseToPtr.h +++ b/DataFormats/Common/interface/RefToBaseToPtr.h @@ -6,8 +6,6 @@ Ref: A function template for conversion from RefToBase to Ptr ----------------------------------------------------------------------*/ -/* - ----------------------------------------------------------------------*/ #include "DataFormats/Common/interface/RefToBase.h" #include "DataFormats/Common/interface/Ptr.h" @@ -43,7 +41,6 @@ namespace edm { // by construction would be persistent return Ptr(); } else { - //Another thread could change this value so get only once EDProductGetter const* getter = ref.productGetter(); if (getter) { return Ptr(ref.id(), ref.key(), getter); From 7c071fc1127fa301ac8571d56df2130e7d011f84 Mon Sep 17 00:00:00 2001 From: Nicholas Smith Date: Tue, 21 Nov 2023 14:40:15 -0600 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Chris Jones --- DataFormats/Common/interface/RefToBaseToPtr.h | 2 +- DataFormats/PatCandidates/src/classes_def_trigger.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DataFormats/Common/interface/RefToBaseToPtr.h b/DataFormats/Common/interface/RefToBaseToPtr.h index 22c6451cfaa12..54d5e444ca4ac 100644 --- a/DataFormats/Common/interface/RefToBaseToPtr.h +++ b/DataFormats/Common/interface/RefToBaseToPtr.h @@ -29,7 +29,7 @@ namespace edm { } /* - * Do not use this ouside an ioread rule in classes_def.xml + * Do not use this outside an ioread rule in classes_def.xml */ template Ptr refToBaseToPtr_ioread(RefToBase const& ref) { diff --git a/DataFormats/PatCandidates/src/classes_def_trigger.xml b/DataFormats/PatCandidates/src/classes_def_trigger.xml index 011b32f1a8dee..9cdb60a549616 100644 --- a/DataFormats/PatCandidates/src/classes_def_trigger.xml +++ b/DataFormats/PatCandidates/src/classes_def_trigger.xml @@ -8,7 +8,7 @@ - + From e4f6639624e65b7bdb5eba19d78bcde756e1f568 Mon Sep 17 00:00:00 2001 From: Nick Smith Date: Tue, 28 Nov 2023 08:17:24 -0600 Subject: [PATCH 7/7] Put RefToBaseToPtr_ioread in separate file --- DataFormats/Common/interface/RefToBaseToPtr.h | 25 ------------- .../Common/interface/RefToBaseToPtr_ioread.h | 37 +++++++++++++++++++ .../PatCandidates/src/classes_trigger.h | 2 +- 3 files changed, 38 insertions(+), 26 deletions(-) create mode 100644 DataFormats/Common/interface/RefToBaseToPtr_ioread.h diff --git a/DataFormats/Common/interface/RefToBaseToPtr.h b/DataFormats/Common/interface/RefToBaseToPtr.h index 54d5e444ca4ac..440c4492b0d2b 100644 --- a/DataFormats/Common/interface/RefToBaseToPtr.h +++ b/DataFormats/Common/interface/RefToBaseToPtr.h @@ -27,30 +27,5 @@ namespace edm { } return Ptr(ref.id(), ref.get(), ref.key()); } - - /* - * Do not use this outside an ioread rule in classes_def.xml - */ - template - Ptr refToBaseToPtr_ioread(RefToBase const& ref) { - if (ref.isNull()) { - return Ptr(); - } - if (ref.isTransient()) { - // This is a logic error, any ref being deserialized - // by construction would be persistent - return Ptr(); - } else { - EDProductGetter const* getter = ref.productGetter(); - if (getter) { - return Ptr(ref.id(), ref.key(), getter); - } - } - // If this is called in an iorule outside the framework, we cannot call - // ref.get(), but since outside the framework we can never fetch the ref, - // the Ptr will only be useful if accessed later from inside the framework. - // We can fill with a nullptr for now - return Ptr(ref.id(), static_cast(nullptr), ref.key()); - } } // namespace edm #endif diff --git a/DataFormats/Common/interface/RefToBaseToPtr_ioread.h b/DataFormats/Common/interface/RefToBaseToPtr_ioread.h new file mode 100644 index 0000000000000..30367854f1679 --- /dev/null +++ b/DataFormats/Common/interface/RefToBaseToPtr_ioread.h @@ -0,0 +1,37 @@ +#ifndef DataFormats_Common_RefToBaseToPtr_ioread_h +#define DataFormats_Common_RefToBaseToPtr_ioread_h + +/*---------------------------------------------------------------------- + +Ref: A function template for conversion from RefToBase to Ptr for use +in ROOT ioread rules for schema migration + +----------------------------------------------------------------------*/ + +#include "DataFormats/Common/interface/RefToBase.h" +#include "DataFormats/Common/interface/Ptr.h" + +namespace edm { + template + Ptr refToBaseToPtr_ioread(RefToBase const& ref) { + if (ref.isNull()) { + return Ptr(); + } + if (ref.isTransient()) { + // This is a logic error, any ref being deserialized + // by construction would be persistent + return Ptr(); + } else { + EDProductGetter const* getter = ref.productGetter(); + if (getter) { + return Ptr(ref.id(), ref.key(), getter); + } + } + // If this is called in an iorule outside the framework, we cannot call + // ref.get(), but since outside the framework we can never fetch the ref, + // the Ptr will only be useful if accessed later from inside the framework. + // We can fill with a nullptr for now + return Ptr(ref.id(), static_cast(nullptr), ref.key()); + } +} // namespace edm +#endif diff --git a/DataFormats/PatCandidates/src/classes_trigger.h b/DataFormats/PatCandidates/src/classes_trigger.h index 10c35dd742450..0debf7671a4f5 100644 --- a/DataFormats/PatCandidates/src/classes_trigger.h +++ b/DataFormats/PatCandidates/src/classes_trigger.h @@ -2,7 +2,7 @@ #include "DataFormats/Common/interface/Association.h" #include "DataFormats/Common/interface/Wrapper.h" #include "DataFormats/Common/interface/PtrVector.h" -#include "DataFormats/Common/interface/RefToBaseToPtr.h" +#include "DataFormats/Common/interface/RefToBaseToPtr_ioread.h" #include "DataFormats/PatCandidates/interface/TriggerObjectStandAlone.h" #include "DataFormats/PatCandidates/interface/TriggerEvent.h"