Skip to content

TPC SCD: round instead of truncate when packing the unbinned residuals #16046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shahor02 merged 2 commits into from
Nov 4, 2026
Merged

TPC SCD: round instead of truncate when packing the unbinned residuals #16046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
faef6b7
TPC SCD: round instead of truncate when packing the unbinned residuals
matthias-kleiner Nov 4, 2026
261ba2d
TPC SCD: rename UnbinnedResid::pack to packToShort, document the fixe…
matthias-kleiner Nov 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions Detectors/TPC/calibration/SpacePoints/include/SpacePoints/TrackInterpolation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#define ALICEO2_TPC_TRACKINTERPOLATION_H_

#include <gsl/span>
#include <algorithm>
#include <cmath>
#include "CommonDataFormat/EvIndex.h"
#include "CommonDataFormat/RangeReference.h"
#include "ReconstructionDataFormats/Track.h"
Expand Down Expand Up @@ -77,11 +79,11 @@ struct TPCClusterResiduals {
struct UnbinnedResid {
UnbinnedResid() = default;
UnbinnedResid(float dyIn, float dzIn, float tgSlpIn, float yIn, float zIn,
unsigned char rowIn, unsigned char secIn, short chanIn = -1, bool rejFlag = false) : dy(static_cast<short>(dyIn * 0x7fff / param::MaxResid)),
dz(static_cast<short>(dzIn * 0x7fff / param::MaxResid)),
tgSlp(static_cast<short>(tgSlpIn * 0x7fff / param::MaxTgSlp)),
y(static_cast<short>(yIn * 0x7fff / param::MaxY)),
z(static_cast<short>(zIn * 0x7fff / param::MaxZ)),
unsigned char rowIn, unsigned char secIn, short chanIn = -1, bool rejFlag = false) : dy(packToShort(dyIn, param::MaxResid)),
dz(packToShort(dzIn, param::MaxResid)),
tgSlp(packToShort(tgSlpIn, param::MaxTgSlp, std::abs(tgSlpIn) < param::MaxTgSlp ? 0x7fff - 1 : 0x7fff)),
y(packToShort(yIn, param::MaxY)),
z(packToShort(zIn, param::MaxZ)),
row(rowIn),
sec(secIn),
channel(chanIn),
Expand All @@ -96,6 +98,16 @@ struct UnbinnedResid {
short channel{-1}; ///< extra channel info (ITS chip ID, TRD chamber, TOF main pad within the sector)
bool rejected{false}; ///< residual is flagged as rejected in the validateTrack

/// value (|value| <= maximum) packed into a signed short, rounded to the nearest step. The scale is fixed at 0x7fff steps per maximum
/// (decoded everywhere as short * maximum / 0x7fff); limit only clips the result to +-limit (e.g. 0x7fff - 1 for an unclamped tgSlp, to
/// keep 0x7fff as the clamp marker), it does not change the scale. A plain cast would truncate towards zero, i.e. move every value
/// towards zero by up to one step: the decoded cluster position y + dy then depends on the sign of dy below the step size, which
/// biases residuals binned by the cluster position on sub-pad scales
static short packToShort(float value, float maximum, int limit = 0x7fff)
{
return static_cast<short>(std::clamp<long>(std::lround(value * 0x7fff / maximum), -limit, limit));

@shahor02 shahor02 Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better do this as

return static_cast<short>(std::clamp<long>(std::lround(value * limit / maximum), -limit, limit));

@matthias-kleiner matthias-kleiner Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For tgSlp the limit is std::abs(tgSlpIn) < param::MaxTgSlp ? 0x7fff - 1 : 0x7fff (unclamped values must not round onto 0x7fff, which isTgSlpClamped() uses as the clamp marker). With limit as the scale, unclamped tgSlp would be packed with 0x7ffe while the decoding uses 0x7fff, a relative difference of 3e-5. Negligible in practice, but packing and decoding would no longer use the same scale. Shall I still change it or keep 0x7fff as the scale and limit only for the clipping?

@shahor02 shahor02 Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, indeed. I just find it confusing that the pack() looks generic, but the limit is intrinsically tied to the scale constant. Perhaps one should simply rename pack to packToUShort

@matthias-kleiner matthias-kleiner Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. I renamed it, thanks!

}

/// true if tgSlp was saturated at +-param::MaxTgSlp (scdcalib.clampTgSlp): unclamped values have |tgSlp| <= 0x7fff - 1
bool isTgSlpClamped() const { return tgSlp == 0x7fff || tgSlp == -0x7fff; }
/// tgSlp marker of a position-only TPC cluster (scdcalib.keepClustersOnPropFail): no reference track at this cluster,
Expand Down
6 changes: 3 additions & 3 deletions Detectors/TPC/calibration/SpacePoints/src/TrackInterpolation.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ void TrackInterpolation::interpolateTrack(int iSeed)
auto dy = yv - trkWorkITS.getY();
auto dz = zv - trkWorkITS.getZ();
if ((std::abs(dy) < param::MaxResid) && (std::abs(dz) < param::MaxResid) && (std::abs(trkWorkITS.getY()) < param::MaxY) && (std::abs(trkWorkITS.getZ()) < param::MaxZ) && std::abs(xv) < param::MaxVtxX) {
short compXV = static_cast<short>(xv * 0x7fff / param::MaxVtxX);
short compXV = UnbinnedResid::packToShort(xv, param::MaxVtxX);
mClRes.emplace_back(dy, dz, alpha / TMath::Pi(), trkWorkITS.getY(), trkWorkITS.getZ(), 190, -1, compXV);
if (!gidTable[GTrackID::ITSTPC].isIndexSet()) {
LOGP(fatal, "ITS-TPC seed index is not set for TOF track");
Expand Down Expand Up @@ -985,7 +985,7 @@ int TrackInterpolation::processTRDLayer(const o2::trd::TrackTRD& trkTRD, int iLa
if (trkData) {
auto slope = trdSP.getDy();
if (std::abs(slope) < param::MaxTRDSlope) {
trkData->TRDTrkltSlope[iLayer] = slope * 0x7fff / param::MaxTRDSlope;
trkData->TRDTrkltSlope[iLayer] = UnbinnedResid::packToShort(slope, param::MaxTRDSlope);
}
}
if (trk64) {
Expand Down Expand Up @@ -1308,7 +1308,7 @@ void TrackInterpolation::extrapolateTrack(int iSeed)
auto dy = yv - trkWorkITS.getY();
auto dz = zv - trkWorkITS.getZ();
if ((std::abs(dy) < param::MaxResid) && (std::abs(dz) < param::MaxResid) && (std::abs(trkWorkITS.getY()) < param::MaxY) && (std::abs(trkWorkITS.getZ()) < param::MaxZ) && std::abs(xv) < param::MaxVtxX) {
short compXV = static_cast<short>(xv * 0x7fff / param::MaxVtxX);
short compXV = UnbinnedResid::packToShort(xv, param::MaxVtxX);
mClRes.emplace_back(dy, dz, alpha / TMath::Pi(), trkWorkITS.getY(), trkWorkITS.getZ(), 190, -1, compXV);
if (!gidTableFull[GTrackID::ITSTPC].isIndexSet()) {
LOGP(fatal, "ITS-TPC seed index is not set for TOF track");
Expand Down
Loading