Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add noise hits to reconstructed hits by randomly generating cell id #1643

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions src/algorithms/tracking/TrackerHitReconstruction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <iterator>
#include <utility>
#include <vector>
#include <random>
#include <bitset> // For displaying bits (optional)

namespace eicrecon {

Expand All @@ -41,6 +43,8 @@ std::unique_ptr<edm4eic::TrackerHitCollection> TrackerHitReconstruction::process

auto rec_hits { std::make_unique<edm4eic::TrackerHitCollection>() };


// add hits from DD4hep
for (const auto& raw_hit : raw_hits) {

auto id = raw_hit.getCellID();
Expand Down Expand Up @@ -85,6 +89,74 @@ std::unique_ptr<edm4eic::TrackerHitCollection> TrackerHitReconstruction::process

}

//------------------------------------------------
// Example code of adding random noise hits
// Shujie Li, 08.2024
// use the first raw hits to obtain the volume ID of the detector system
// this also make sure the hit container has at least one hit
Comment on lines +92 to +96
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//------------------------------------------------
// Example code of adding random noise hits
// Shujie Li, 08.2024
// use the first raw hits to obtain the volume ID of the detector system
// this also make sure the hit container has at least one hit
// Use the first raw hits to obtain the volume ID of the detector system.
// This also make sure the hit container has at least one hit.

And add your name on the second line of the file.

Comment on lines +95 to +96
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something that you really want or just a consequence of how you're getting the volume? Might it not be better to have a completely separate algorithm which constructs these hits from knowledge of the readout structure to get the volume rather than relying on their being a hit.

int num_hits=10; // total number of noise hits in this detector volume. Should move this to the config file
std::vector<uint64_t> noise_ids;

// generate valid random cell id
for (const auto& hit0 : raw_hits) {
uint64_t id0,vol_id;
id0 = hit0.getCellID();
// vol ID is the last 8 bits in Si tracker. Need to make it more flexible
// may want to predefine the layer/module ID as well to speed up the radom ID generation
vol_id = id0 & 0xFF;
Comment on lines +102 to +106
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not call it "system ID"?

Suggested change
uint64_t id0,vol_id;
id0 = hit0.getCellID();
// vol ID is the last 8 bits in Si tracker. Need to make it more flexible
// may want to predefine the layer/module ID as well to speed up the radom ID generation
vol_id = id0 & 0xFF;
uint64_t id0, system_id;
id0 = hit0.getCellID();
// system ID is the last 8 bits in Si tracker. Need to make it more flexible
// may want to predefine the layer/module ID as well to speed up the radom ID generation
system_id = id0 & 0xFF;

// std::cout<<"::: "<<raw_hits.size()<<"/"<<id0<<"=="<<std::bitset<8>(id0)<<" ::"<<vol_id<<std::endl;

// Setup random number generator
std::random_device rd; // Obtain a random number from hardware
std::mt19937_64 eng(rd()); // Seed the generator
std::uniform_int_distribution<uint64_t> distr; // Define the range for 64-bit integers
Comment on lines +110 to +112
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not create a new random device for every hit.

dd4hep::Position pos;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dd4hep::Position pos;

int nn=0;
int i=0;
Comment on lines +114 to +115
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please give clear name to the variables.

Suggested change
int nn=0;
int i=0;
unsigned int num_failures = 0;
unsigned int num_generated_hits = 0;

while (i < num_hits) {
uint64_t randomNum = distr(eng); // Generate a random 64-bit number
randomNum = (randomNum & ~uint64_t(0xFF)) | vol_id; // Clear the last 8 bits and set them to 'vol ID'
Comment on lines +117 to +118
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this is clear and simple, but is performance terrible or not?

try {
pos = m_converter->position(randomNum);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pos = m_converter->position(randomNum);
auto pos = m_converter->position(randomNum);

// std::cout<<" ::: converter position "<<pos.x()<<std::endl;
noise_ids.push_back(randomNum);
i++;
} catch(std::exception &e) {
// std::cout<<"::: cell ID error caught"<<std::endl;
nn++;
}
std::cout <<std::bitset<8>(vol_id) <<":"<< i<<"/"<<nn<<":::"<<randomNum<<" ";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove "cout" output (even commented), or replace with clearly worded debug logging.

}
break;
}

// generate noise hits from ids.
for (auto id : noise_ids) {
// Get position and dimension
auto pos = m_converter->position(id);
auto dim = m_converter->cellDimensions(id);
// >oO trace
if(m_log->level() == spdlog::level::trace) {
m_log->trace("Noise hits inserted: position x={:.2f} y={:.2f} z={:.2f} [mm]: ", pos.x()/ mm, pos.y()/ mm, pos.z()/ mm);
m_log->trace("dimension size: {}", dim.size());
for (size_t j = 0; j < std::size(dim); ++j) {
m_log->trace(" - dimension {:<5} size: {:.2}", j, dim[j]);
}
}
#if EDM4EIC_VERSION_MAJOR >= 7
auto rec_hit =
#endif
Comment on lines +146 to +148
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#if EDM4EIC_VERSION_MAJOR >= 7
auto rec_hit =
#endif

Not needed if you don't need to set a relation.

rec_hits->create(
id, // Raw DD4hep cell ID
edm4hep::Vector3f{static_cast<float>(pos.x() / mm), static_cast<float>(pos.y() / mm), static_cast<float>(pos.z() / mm)}, // mm
edm4eic::CovDiag3f{get_variance(dim[0] / mm), get_variance(dim[1] / mm), // variance (see note above)
std::size(dim) > 2 ? get_variance(dim[2] / mm) : 0.},
static_cast<float>(0), // ns
m_cfg.timeResolution, // in ns
static_cast<float>(0), // Collected energy (GeV)
0.0F); // Error on the energy

}
return std::move(rec_hits);
}

Expand Down
Loading