Implemented improvements:
Now, there's the parameter charge_weighting to choose between charge weighted and arithmetic mean cluster centre calculation.
I also changed the Detector member functions getLocalPosition() and so on because:
it would be much better to define a pixel as [col-0.5,col+0.5) rather than [col,col+1) as it is mostly (but not completely consistently) done now. This was fixed in a separate merge request (!116 (merged)), also updated all histograms etc. which are affected by this change.
Another issue we should discuss:
We have a couple of pixel hits with charge=0. When these build single-pixel clusters, the charge-weighted mean will fail because
column += (pixel->column() * pixel->charge());
row += (pixel->row() * pixel->charge());and later
column /= (charge > std::numeric_limits<double>::epsilon() ? charge : 1);
row /= (charge > std::numeric_limits<double>::epsilon() ? charge : 1);don't make sense as we do something like column = 0/1. However, if I add a condition charge!=0 here, the charge-weighted mean is disturbed for multi-pixel clusters containing 1 pixel with zero charge.
Note: Previously, pixels with zero charge were completely kicked out.
How should we handle that?
Update:
[See discussion below] The above problem, i.e. how to handle pixels with zero charge, is solved now by the following: If a cluster contains a pixel with zero charge we fall back to arithmetic mean instead of charge-weighted centre-of-gravity - assuming that charge=0 doesn't mean tiny charge but rather unknown charge, such that the arithmetic mean is the better estimate for the cluster position.
To Do before merging:
-
remove debug histograms -
solve single-pixel with charge=0 question (see description above) -
Implement the same in ClusteringSpatial()