fix: Duplicate layers in HoughCell #4987
Conversation
| /// a batch to resize the vector of the hits | ||
| static constexpr std::size_t m_assignBatch{20}; |
There was a problem hiding this comment.
I don't see advantage of enforcing the assignment at compile time.
| std::span<const identifier_t, std::dynamic_extent> | ||
| Acts::HoughTransformUtils::HoughCell<identifier_t>::getHits() const { | ||
| std::span<const identifier_t, std::dynamic_extent> hits(m_hits.begin(), | ||
| m_iHit); |
There was a problem hiding this comment.
The idea was to reuse the same hough cell without reallocating the memory
| m_hits.clear(); | ||
| m_layers.clear(); |
There was a problem hiding this comment.
The index manipulation was developed because we saw it performed better. @dimitra97
| // Check for duplicates | ||
| if (std::find(m_layers.begin(), m_layers.end(), layer) == m_layers.end()) { | ||
| m_layers.push_back(layer); | ||
| m_nLayers += weight; | ||
| } |
There was a problem hiding this comment.
I foree here already a big CPU bottleneck. Why not checking that the last layer numer corresponds to the current one. Same logic as for the hit filling?
| for (uint8_t layer = 1; layer <= nLayers; ++layer) { | ||
| // Add hits equal to the layer number | ||
| for (uint8_t hit = 0; hit < layer; ++hit) { | ||
| plane.fillBin(0, 0, nHits++, layer); |
There was a problem hiding this comment.
Here you loop layer by layer. There's'no need to loop through the entire vector of [0-8] if you're filling layer 9. Just check whether the last layer number is different. If people are jumping between the layers, they must do the duplicate removal themselves in the peak finder!
|
3 participants
This PR fixes duplicate layers in
HoughCellobject:m_layerandm_hitsas vectors, but make some changes how they are handled:m_hits, instead of callingresizewhen nearing the capacity of the vectors, simply callreserve.m_layersthere is no need for such treatment, as number of layers should be small. To avoid making it into astd::set, add extra check to make sure that duplicates are not inserted.std::vector::end()instead (for member functions returningstd::span).--- END COMMIT MESSAGE ---
Tagging @goblirsc and @junggjo9 since this code is used by muons (also in Athena), but since this was unnoticed, I guess it was not used for anything critical.