[PWGCF] Addition of resonance correlations #13422
Conversation
|
O2 linter results: ❌ 0 errors, |
github-actions
bot
changed the title
[PWGCF] Please consider the following formatting changes to AliceO2Group#13422
Preet-Bhanjan
requested review from
alibuild,
ariedel-cern,
glromane,
lauraser,
lgraczykCern,
otonvd,
prchakra,
shouqiye,
victor-gonzalez and
zchochul
as code owners
| bool isPion, isKaon, isProton; | ||
| bool isDetectedPion = nSigmaToUse[0] < detectorNsigmaCut[0] && nSigmaToUse[0] > detectorNsigmaCut[0 + 3]; | ||
| bool isDetectedKaon = nSigmaToUse[1] < detectorNsigmaCut[1] && nSigmaToUse[1] > detectorNsigmaCut[1 + 3]; | ||
| bool isDetectedProton = nSigmaToUse[2] < detectorNsigmaCut[2] && nSigmaToUse[2] > detectorNsigmaCut[2 + 3]; | ||
|
|
||
| bool isTofPion = nSigmaTOF[0] < tofNsigmaCut[0] && nSigmaTOF[0] > tofNsigmaCut[0 + 3]; | ||
| bool isTofKaon = nSigmaTOF[1] < tofNsigmaCut[1] && nSigmaTOF[1] > tofNsigmaCut[1 + 3]; | ||
| bool isTofProton = nSigmaTOF[2] < tofNsigmaCut[2] && nSigmaTOF[2] > tofNsigmaCut[2 + 3]; | ||
|
|
There was a problem hiding this comment.
Please, use named constants instead of 'magic' number as indexes
There was a problem hiding this comment.
Thanks for pointing it out. I will be using named constants from now on
| if (isPion) { | ||
| pid = PIONS; | ||
| } else if (isKaon) { | ||
| pid = KAONS; | ||
| } else if (isProton) { | ||
| pid = PROTONS; | ||
| } else { | ||
| return 0; // no particle satisfies the criteria | ||
| } | ||
|
|
||
| return pid + 1; // shift the pid by 1, 1 = pion, 2 = kaon, 3 = proton |
There was a problem hiding this comment.
This is really confusing and error prone
If a pion has pid as PIONS it does not make too much sense that is also PIONS+1 because then PIONS is pions and also PIONS+1 is pions
It is confusing even writing it
There was a problem hiding this comment.
The idea was to use PIONS, KAONS and PROTONS as indices for arrays/configurable arrays and have the getNsigmaPID to generate pid for pions, kaons, and protons as 1, 2, 3 (0 for not identified). I see it now that its confusing. I will modify it so that pid = PIONS(KAONS, etc) + 1 . I hope this will make it more readable and clear
| template <typename T> | ||
| auto projectMatrix(Array2D<T> const& mat, std::array<float, 6>& array1, std::array<float, 6>& array2, std::array<float, 6>& array3) | ||
| { | ||
| for (auto j = 0; j < static_cast<int>(mat.cols); ++j) { | ||
| array1[j] = mat(0, j); | ||
| array2[j] = mat(1, j); | ||
| array3[j] = mat(2, j); | ||
| } | ||
| return; | ||
| } | ||
| template <typename T, typename P> | ||
| auto readMatrix(Array2D<T> const& mat, P& array) | ||
| { | ||
| for (auto i = 0; i < static_cast<int>(mat.rows); ++i) { | ||
| for (auto j = 0; j < static_cast<int>(mat.cols); ++j) { | ||
| array[i][j] = mat(i, j); | ||
| } | ||
| } | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
This is really dangerous
The vectors structures does not have checks on their range which means that if they read or written out of their bounds most of the times it will not be noticed but other data is being read or overwritten which is really hard to fix and produces random results
Will it not work passing references to empty arrays of the right type and then filling the array with the corresponding elements?
Also named constants can be used to directly access the elements of the matrix without the need of serializing its content
|
This pull request got closed accidentally as I pushed new changes after rebuilding O2. |
2 participants
No description provided.