Skip to content

Commit

Permalink
algorithms/calorimetry/CalorimeterHitDigi: pass PODIO collections ins…
Browse files Browse the repository at this point in the history
…tead of std::vector

This fixes memory leaks like:

```
Direct leak of 407056 byte(s) in 50882 object(s) allocated from:
    #0 0x55e48bee007d in operator new(unsigned long) (/home/runner/work/EICrecon/EICrecon/bin/eicrecon+0xeb07d) (BuildId: 5069f7d2185cef71541ac1e93e2f6643618de2c4)
    #1 0x7fa316881fc5 in CalorimeterHitDigi::signal_sum_digi() /home/runner/work/EICrecon/EICrecon/src/algorithms/calorimetry/CalorimeterHitDigi.cc:254:23
    #2 0x7fa3168807eb in CalorimeterHitDigi::AlgorithmProcess() /home/runner/work/EICrecon/EICrecon/src/algorithms/calorimetry/CalorimeterHitDigi.cc:127:9
    #3 0x7fa3150b2e0c in RawCalorimeterHit_factory_LFHCALRawHits::Process(std::shared_ptr<JEvent const> const&) /home/runner/work/EICrecon/EICrecon/src/detectors/FHCAL/RawCalorimeterHit_factory_LFHCALRawHits.h:86:9
    #4 0x7fa315a0fcc1 in eicrecon::JFactoryPodioT<edm4hep::RawCalorimeterHit>::Create(std::shared_ptr<JEvent const> const&) /home/runner/work/EICrecon/EICrecon/src/services/io/podio/JFactoryPodioT.h:171:13
    #5 0x7fa315a4e8a8 in JFactoryT<edm4hep::RawCalorimeterHit>::GetOrCreate(std::shared_ptr<JEvent const> const&) /opt/software/linux-debian12-x86_64_v2/gcc-12.2.0/jana2-2.1.0-hj25lfdff2mcbllcvr5lojwa4sqpnpb5/include/JANA/JFactoryT.h:83:13
    #6 0x7fa315a4d87b in std::vector<edm4hep::RawCalorimeterHit const*, std::allocator<edm4hep::RawCalorimeterHit const*> > JEvent::Get<edm4hep::RawCalorimeterHit>(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const /opt/software/linux-debian12-x86_64_v2/gcc-12.2.0/jana2-2.1.0-hj25lfdff2mcbllcvr5lojwa4sqpnpb5/include/JANA/JEvent.h:325:27
```
  • Loading branch information
veprbl committed Jul 5, 2023
1 parent 7596c65 commit 25d5609
Show file tree
Hide file tree
Showing 17 changed files with 105 additions and 140 deletions.
55 changes: 20 additions & 35 deletions src/algorithms/calorimetry/CalorimeterHitDigi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "CalorimeterHitDigi.h"

#include <JANA/JEvent.h>
#include <edm4hep/SimCalorimeterHit.h>
#include <Evaluator/DD4hepUnits.h>
#include <fmt/format.h>
using namespace dd4hep;
Expand Down Expand Up @@ -116,29 +115,21 @@ void CalorimeterHitDigi::AlgorithmChangeRun() {
//------------------------
// AlgorithmProcess
//------------------------
void CalorimeterHitDigi::AlgorithmProcess() {

// Delete any output objects left from last event.
// (Should already have been done for us, but just to be bullet-proof.)
for( auto h : rawhits ) delete h;
rawhits.clear();

std::unique_ptr<edm4hep::RawCalorimeterHitCollection> CalorimeterHitDigi::AlgorithmProcess(const edm4hep::SimCalorimeterHitCollection &simhits) {
if (merge_hits) {
signal_sum_digi();
return std::move(signal_sum_digi(simhits));
} else {
single_hits_digi();
return std::move(single_hits_digi(simhits));
}
}

//------------------------
// single_hits_digi
//------------------------
void CalorimeterHitDigi::single_hits_digi(){
std::unique_ptr<edm4hep::RawCalorimeterHitCollection> CalorimeterHitDigi::single_hits_digi(const edm4hep::SimCalorimeterHitCollection &simhits) {
std::unique_ptr<edm4hep::RawCalorimeterHitCollection> rawhits { std::make_unique<edm4hep::RawCalorimeterHitCollection>() };

// Create output collections
for ( auto ahit : simhits ) {
// Note: juggler internal unit of energy is dd4hep::GeV
const double eDep = ahit->getEnergy();
const double eDep = ahit.getEnergy();

// apply additional calorimeter noise to corrected energy deposit
const double eResRel = (eDep > m_threshold)
Expand All @@ -157,7 +148,7 @@ void CalorimeterHitDigi::single_hits_digi(){
const long long adc = std::llround(ped + eDep * (m_corrMeanScale + eResRel) / m_dyRangeADC * m_capADC);

double time = std::numeric_limits<double>::max();
for (const auto& c : ahit->getContributions()) {
for (const auto& c : ahit.getContributions()) {
if (c.getTime() <= time) {
time = c.getTime();
}
Expand All @@ -166,36 +157,29 @@ void CalorimeterHitDigi::single_hits_digi(){

const long long tdc = std::llround((time + m_normDist(generator) * tRes) * stepTDC);

if (eDep> 1.e-3) m_log->trace("E sim {} \t adc: {} \t time: {}\t maxtime: {} \t tdc: {} \t cell ID {}", eDep, adc, time, m_capTime, tdc, ahit->getCellID());
auto rawhit = new edm4hep::RawCalorimeterHit(
ahit->getCellID(),
if (eDep> 1.e-3) m_log->trace("E sim {} \t adc: {} \t time: {}\t maxtime: {} \t tdc: {} \t cell ID {}", eDep, adc, time, m_capTime, tdc, ahit.getCellID());
rawhits->create(
ahit.getCellID(),
(adc > m_capADC ? m_capADC : adc),
tdc
);
rawhits.push_back(rawhit);
}

return std::move(rawhits);
}

//------------------------
// signal_sum_digi
//------------------------
void CalorimeterHitDigi::signal_sum_digi( void ){
std::unique_ptr<edm4hep::RawCalorimeterHitCollection> CalorimeterHitDigi::signal_sum_digi(const edm4hep::SimCalorimeterHitCollection &simhits) {
std::unique_ptr<edm4hep::RawCalorimeterHitCollection> rawhits { std::make_unique<edm4hep::RawCalorimeterHitCollection>() };

// find the hits that belong to the same group (for merging)
std::unordered_map<long long, std::vector<const edm4hep::SimCalorimeterHit*>> merge_map;
for (auto ahit : simhits) {
int64_t hid = ahit->getCellID() & id_mask;
int64_t hid = ahit.getCellID() & id_mask;

m_log->trace("org cell ID in {:s}: {:#064b}", m_readout, ahit->getCellID());
m_log->trace("org cell ID in {:s}: {:#064b}", m_readout, ahit.getCellID());
m_log->trace("new cell ID in {:s}: {:#064b}", m_readout, hid);

auto it = merge_map.find(hid);

if (it == merge_map.end()) {
merge_map[hid] = {ahit};
} else {
it->second.push_back(ahit);
}
merge_map[hid].push_back(&ahit);
}

// signal sum
Expand Down Expand Up @@ -251,11 +235,12 @@ void CalorimeterHitDigi::signal_sum_digi( void ){
unsigned long long tdc = std::llround((time + m_normDist(generator) * tRes) * stepTDC);

if (edep> 1.e-3) m_log->trace("E sim {} \t adc: {} \t time: {}\t maxtime: {} \t tdc: {}", edep, adc, time, m_capTime, tdc);
auto rawhit = new edm4hep::RawCalorimeterHit(
rawhits->create(
mid,
(adc > m_capADC ? m_capADC : adc),
tdc
);
rawhits.push_back(rawhit);
}

return std::move(rawhits);
}
16 changes: 6 additions & 10 deletions src/algorithms/calorimetry/CalorimeterHitDigi.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@

#pragma once

#include <memory>
#include <random>

#include <services/geometry/dd4hep/JDD4hep_service.h>

#include <edm4hep/SimCalorimeterHit.h>
#include <edm4hep/RawCalorimeterHit.h>
#include <edm4hep/SimCalorimeterHitCollection.h>
#include <edm4hep/RawCalorimeterHitCollection.h>
#include <spdlog/spdlog.h>

class CalorimeterHitDigi {
Expand All @@ -27,10 +28,9 @@ class CalorimeterHitDigi {

public:
CalorimeterHitDigi() = default;
~CalorimeterHitDigi(){for( auto h : rawhits ) delete h;} // better to use smart pointer?
virtual void AlgorithmInit(std::shared_ptr<spdlog::logger>& logger);
virtual void AlgorithmChangeRun() ;
virtual void AlgorithmProcess() ;
virtual std::unique_ptr<edm4hep::RawCalorimeterHitCollection> AlgorithmProcess(const edm4hep::SimCalorimeterHitCollection &simhits) ;

//-------- Configuration Parameters ------------
//instantiate new spdlog logger
Expand Down Expand Up @@ -78,14 +78,10 @@ class CalorimeterHitDigi {
std::shared_ptr<JDD4hep_service> m_geoSvc;
uint64_t id_mask{0};

// inputs/outputs
std::vector<const edm4hep::SimCalorimeterHit*> simhits;
std::vector<edm4hep::RawCalorimeterHit*> rawhits;

private:
std::default_random_engine generator; // TODO: need something more appropriate here
std::normal_distribution<double> m_normDist; // defaults to mean=0, sigma=1

void single_hits_digi();
void signal_sum_digi();
std::unique_ptr<edm4hep::RawCalorimeterHitCollection> single_hits_digi(const edm4hep::SimCalorimeterHitCollection &simhits);
std::unique_ptr<edm4hep::RawCalorimeterHitCollection> signal_sum_digi(const edm4hep::SimCalorimeterHitCollection &simhits);
};
11 changes: 5 additions & 6 deletions src/detectors/B0ECAL/RawCalorimeterHit_factory_B0ECalRawHits.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,14 @@ class RawCalorimeterHit_factory_B0ECalRawHits : public eicrecon::JFactoryPodioT<
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,14 @@ class RawCalorimeterHit_factory_EcalBarrelImagingRawHits : public eicrecon::JFac
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,14 @@ class RawCalorimeterHit_factory_EcalBarrelScFiRawHits : public eicrecon::JFactor
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,14 @@ class RawCalorimeterHit_factory_EcalBarrelSciGlassRawHits : public eicrecon::JFa
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,14 @@ class RawCalorimeterHit_factory_HcalBarrelRawHits : public eicrecon::JFactoryPod
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,14 @@ class RawCalorimeterHit_factory_EcalEndcapNRawHits : public eicrecon::JFactoryPo
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,14 @@ class RawCalorimeterHit_factory_HcalEndcapNRawHits : public eicrecon::JFactoryPo
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,14 @@ class RawCalorimeterHit_factory_EcalEndcapPInsertRawHits : public eicrecon::JFac
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,14 @@ class RawCalorimeterHit_factory_EcalEndcapPRawHits : public eicrecon::JFactoryPo
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,14 @@ class RawCalorimeterHit_factory_HcalEndcapPInsertRawHits : public eicrecon::JFac
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,14 @@ class RawCalorimeterHit_factory_HcalEndcapPRawHits : public eicrecon::JFactoryPo
//------------------------------------------
// Process
void Process(const std::shared_ptr<const JEvent> &event) override {
// Prefill inputs
simhits = event->Get<edm4hep::SimCalorimeterHit>(m_input_tag);
// Get input collection
auto simhits_coll = static_cast<const edm4hep::SimCalorimeterHitCollection*>(event->GetCollectionBase(m_input_tag));

// Call Process for generic algorithm
AlgorithmProcess();
auto rawhits_coll = AlgorithmProcess(*simhits_coll);

// Hand owner of algorithm objects over to JANA
Set(rawhits);
rawhits.clear(); // not really needed, but better to not leave dangling pointers around
// Hand algorithm objects over to JANA
SetCollection(std::move(rawhits_coll));
}

};
Loading

0 comments on commit 25d5609

Please sign in to comment.