PWGCF: FemtoUniverse track-V0 task -- fix for track nSigma binning #7192
Conversation
Eloviyo
requested review from
alibuild,
saganatt,
victor-gonzalez and
zchochul
as code owners
| @@ -1051,7 +1051,7 @@ struct femtoUniverseProducerTask { | |||
| } | |||
|
|
|||
| template <typename TrackType, bool transientLabels = false> | |||
| void fillParticles(TrackType const& tracks) | |||
| void fillParticles(TrackType const& tracks, std::set<int>* recoMcIds = 0) | |||
There was a problem hiding this comment.
- Don't use raw pointers in modern C++ and in O2Physics! They are rarely justified
- 0 is not a proper default value for a pointer in C++.
- Option 1: You should rather use std::optional with default value std::nullopt. Use reference to avoid copying, and use
constto mark that the set is not changed infillParticles(). Like:
void fillParticles(..., std::optional<std::reference_wrapper<const std::set<int>>> recoMcIds = std::nullopt)
line 1077: if(recoMcIds && recoMcIds->get().contains(...))
line 1299: you pas recoMcIds without the &.
4. Option 2, a much better one: would it be a big design change to iterate over tracks in fillParticles(), take MC particle corresponding to the track, and do the rest of fillParticles() with the properties of the obtained MC particle?
You would have no need to create the recoMcIds set, which would also consume a lot of memory and potentially slow down the task, if you overfill CPU cache.
There was a problem hiding this comment.
Thanks a lot for the review, comments and suggestions. I have implemented the option 1 for now as I am in a hurry to produce results for the approval. Option 2 may require me to consult with the group before making a PR, so I shall take a look at it after the approval period.
Head branch was pushed to by a user without write access
…liceO2Group#7192) * fix for track nSigma binning * added a check if reco particles have corresponding mcparticles * requested modification applied * clang fix --------- Co-authored-by: Shirajum Monira <shirajum.monira@cernch>
…liceO2Group#7192) * fix for track nSigma binning * added a check if reco particles have corresponding mcparticles * requested modification applied * clang fix --------- Co-authored-by: Shirajum Monira <shirajum.monira@cernch>
No description provided.