Add cluster shape variables #79
Conversation
|
Thanks @giovannimarchiori . I pushed a new commit to fix these issues. |
|
Related PR in LAr_scripts: |
|
Hi @dasphy , any news on this? |
|
Hi @BrieucF , thanks. More shape variables added, will push the updated code later today |
|
Thank you @kjvbrt , I updated the branch and switched to |
| theta2_E_layer[layer+startPositionToFill] += theta_id * theta_id * eCell; | ||
| theta_E_layer[layer+startPositionToFill] += theta_id * eCell; |
There was a problem hiding this comment.
Since the theta response is better with logE weights, have you tried to calculate the w_theta from the mean theta and theta2 using the logE weights instead of the Ecell weights?
| if (std::isnan(_w_theta_3Bin)) _w_theta_3Bin = 0.; | ||
| width_theta_3Bin[layer+startPositionToFill] = _w_theta_3Bin; | ||
| if (std::isnan(_w_theta_5Bin)) _w_theta_5Bin = 0.; | ||
| width_theta_5Bin[layer+startPositionToFill] = _w_theta_5Bin; | ||
| if (std::isnan(_w_theta_7Bin)) _w_theta_7Bin = 0.; | ||
| width_theta_7Bin[layer+startPositionToFill] = _w_theta_7Bin; | ||
| if (std::isnan(_w_theta_9Bin)) _w_theta_9Bin = 0.; | ||
| width_theta_9Bin[layer+startPositionToFill] = _w_theta_9Bin; |
There was a problem hiding this comment.
does it ever happen that the energy sums in 3-5-7-9 bins are zero? This should not happen as long as you have a maximum energy cell with E>0... I'd rather check this or that the energy sums are zero, rather than correcting the nan shower shapes later (they could be nan due to some other bugs and we would not be able to figure out)
|
Hi @dasphy , I finally managed to went through the full code once more after the latest changes. |
Co-authored-by: Giovanni Marchiori <[email protected]>
Thanks @giovannimarchiori for reviewing the code ! |
|
Hi @dasphy, do you plan to implement the logE weights in this PR? |
|
Hi @kjvbrt @giovannimarchiori |
| @@ -664,6 +664,7 @@ StatusCode AugmentClustersFCCee::execute([[maybe_unused]] const EventContext &ev | |||
| double theta2_E_3Bin = 0.; | |||
| double theta_E_3Bin = 0.; | |||
| double sum_E_3Bin = 0.; | |||
| double sum_weightLog_3Bin = 0.; | |||
There was a problem hiding this comment.
why declaring/calculating the wightlog variables here and not later inside the if (m_do_widthTheta_logE_weights) {..} block? Maybe the compiler is smart enough to optimise this code, but it makes anyway the code logic a bit harder to follow.
| sum_weightLog_3Bin = weightLog_E_max + weightLog_m1 + weightLog_p1; | ||
| sum_weightLog_5Bin = sum_weightLog_3Bin + weightLog_m2 + weightLog_p2; | ||
| sum_weightLog_7Bin = sum_weightLog_5Bin + weightLog_m3 + weightLog_p3; | ||
| sum_weightLog_9Bin = sum_weightLog_7Bin + weightLog_m4 + weightLog_p4; |
There was a problem hiding this comment.
Same comment as for L667.
And the previous 4 lines (sum_E_3bin = ...) could go inside the else { .. }. block of L787
|
Nice, merging... |
Hi all,
Following PR #72 , more shape variables added for clusters.
These variables showed good separations for photon/pi0 identification.
@BrieucF , @kjvbrt
Tagging @giovannimarchiori , @AlexisMlz , @gartrog
Cheers, Tong