Fix GPUTPCGeometry LinearY2PadC and LinearPad2YC, define biased ones in GPUTPCCompressionTrackModel

[shahor02]

@davidrohr @wiechula we had a discussion with Alex about pad<->coordinate conversion, looks like biased cluster coordinates are used only for track-based compression, where the bias is irrelevant (though, in the absence of other distortions, potentially may increase the entropy). Otherwise, the conversion with TPCFastTransform is used, which is fine.
But the inverse, cluster coordinate to pad conversion tpcGeometry.LinearY2Pad, biases the pad value by 0.5, and since it is always rounded to int using GPUCommonMath::Float2UIntRn(float x) { return (uint32_t)(int32_t)(x + 0.5f); }, it effectively shifts the cluster central padID by 1.
This method is used in the tracking only to check if the pad is dead, in the
(1) in the GPUTracking/Merger/GPUTPCGMTrackParam.cxx to decide if the dedx should be calculated
(2) in the GPUTracking/SliceTracker/GPUTPCTrackletConstructor.cxx to decide if in case of not finding a cluster at the probed padrow the tracklet should be penalized for the hole.

This PR adds an extra unbiased method LinearY2PadC and LinearPad2YC, and uses it (1) and (2).
Reprocessing ~60 TFs of LHC24ar_559781 with this PR I don't see any effect on tracking (see the plot below). Cannot judge about the dedx change, if needed, can provide tpctracks.root trees.
Let me know if you still want to merge this.

@wiechula , the LinearPad2Y is also used in the

AliceO2/Detectors/TPC/calibration/src/CorrectdEdxDistortions.cxx

Lines 79 to 87 in 98dc521

	float o2::tpc::CorrectdEdxDistortions::getCorrection(const float time, unsigned char sector, unsigned char padrow, int pad) const
	{
	//
	// Get the corrections at the previous and next padrow and interpolate them to the start and end position of current pad
	// Calculate from corrected position the radial distortions and compare the effective length of distorted electrons with pad length
	//
	// localY of current pad
	const float ly = mTPCGeometry.LinearPad2Y(sector, padrow, pad);

, used to account for the corrections in dEdX calculations, do you want to change it to LinearY2PadC also there?

@davidrohr in the

AliceO2/Detectors/TPC/workflow/src/EntropyEncoderSpec.cxx

Lines 235 to 237 in 98dc521

	const float y = mParam->tpcGeometry.LinearPad2Y(i, j, cl.getPad());
	const float r = sqrtf(x * x + y * y);
	const float maxz = r * mEtaFactor + mMaxZ;

the LinearPad2Y as a marginal correction in the check of cluster timebin coung outside of the allowed range, should we change it also there?

[github-actions]

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass4
async-2023-pp-apass4
async-2024-pp-apass1
async-2022-pp-apass7
async-2024-pp-cpass0
async-2024-PbPb-apass1
async-2024-ppRef-apass1

[davidrohr]

@shahor02 : In principal this PR looks fine. However, are you sure TPCFastTransform is correct?

AliceO2/GPU/TPCFastTransformation/TPCFastTransform.h

Line 361 in c10424c

u = (pad - 0.5f * rowInfo.maxPad) * rowInfo.padWidth;

assumes that the pad coordinate is in the center of the pad, not at the left edge, or do I miss something?
I think in the tracking we were assuming pad coordinates are in the center of the pad, i.e. a single-pad cluster in the first pad would be 0.5. But the clusterizer places the clusters at the left edge, so at 0 not 0.5.
I am not sure where we want to change it.

[shahor02]

@davidrohr yes, the TPCFastTransform is correct: (from MM exchange with Alex):
the transformation is essentially (modulo corrections and sign flip for the TPC side)

AliceO2/GPU/TPCFastTransformation/TPCFastTransform.h

Line 361 in 98dc521

u = (pad - 0.5f * rowInfo.maxPad) * rowInfo.padWidth;

Note that what is subtracted is not the number of pads but the maxpad = NPads - 1. So, effectively, the shift from the left edge to the center is done in the convPadTimeToUV:

u = (pad + 0.5 - NPads/2.) * padWidth

Here is the dump for u vs pad for single-pad clusters :

tt->Scan("u : pad","(pad==0 || pad==maxpad) && row==0")
************************************
*    Row   *        u  *       pad *
************************************
*     2057 * 13.520000 *        65 *
*     8353 * 13.520000 *        65 *
*     9448 * -13.52000 *         0 *
*    19835 * 13.520000 *        65 *
*    34230 * -13.52000 *         0 *
*    48484 * 13.520000 *        65 *
*    52764 * 13.520000 *        65 *
...

tt->Scan("u : pad","(pad==0 || pad==maxpad) && row==150")
************************************
*    Row   *        u  *       pad *
************************************
*    17930 * -41.57899 *         0 *
*    34282 * -41.57899 *         0 *
*    70429 * 41.578998 *       137 *
*   117654 * -41.57899 *         0 *
*   161222 * -41.57899 *         0 *
*   181873 * 41.578998 *       137 *
*   220520 * 41.578998 *       137 *
...

and here is the result of tt->Draw("u >> hh0(10000,-15,15)","row==0"), as you can see, it is symmetric around 0.

For completeness: u corresponds to that printed with the patch:

git diff
diff --git a/GPU/TPCFastTransformation/TPCFastTransform.h b/GPU/TPCFastTransformation/TPCFastTransform.h
index 80c8a04f84..eb3b34d30e 100644
--- a/GPU/TPCFastTransformation/TPCFastTransform.h
+++ b/GPU/TPCFastTransformation/TPCFastTransform.h
@@ -610,11 +610,13 @@ GPUdi() void TPCFastTransform::Transform(int32_t slice, int32_t row, float pad,
   x = rowInfo.x;
   float u = 0, v = 0;
   convPadTimeToUV(slice, row, pad, time, u, v, vertexTime);
+  float u0 = u;
 
   TransformInternal(slice, row, u, v, x, ref, ref2, scale, scale2, scaleMode);
 
   getGeometry().convUVtoLocal(slice, u, v, y, z);
-
+  const TPCFastTransformGeo::RowInfo& rowInfo = getGeometry().getRowInfo(row);
+  printf("%d %.3f %.3f %.3f %.3f   %.2f %.3f %d\n",row, pad, u0, u, y, rowInfo.x, rowInfo.padWidth, rowInfo.maxPad);
   float dzTOF = 0;
   getTOFcorrection(slice, row, x, y, z, dzTOF);
   z += dzTOF;

and read back from the dump with

tt = new TNtuple("tt","","row:pad:u0:u:y:x:w:maxpad");
tt->ReadFile("dmpr.txt");

[davidrohr]

Hi @shahor02 : you are right, I confused maxPad and nPads, as did Jens who send me the link to the code :).
In that case we should leave the clusters as they are, and change the tracking code.
However, I would do 3 changes compared to your current PR:

1. I would copy the old LinearPad2Y and LinearY2Pad

    const float u = (pad - 0.5f * mNPads[row]) * PadWidth(row);
    return (slice >= GPUCA_NSLICES / 2) ? -u : u;

to GPUTPCCompressionTrackModel, using only NPads() and PadWidth() from GPUTPCGeometry, and use that everywhere in GPU/GPUTracking/DataCompression.
I think it is much cleaner since then the convention is part of the compression track model.

2. I would not introduce new functions LinearPad2YC, but fix the existing ones, which will also be cleaner.
3. I would like to verify how the convention was in AliRoot, since the tracking code can also process Run2 data. If, as I remember, we had a 0.5 shift in the cluster pad value there, I'd like to use

#ifdef GPUCA_TPC_GEOMETRY_O2
... mNPads[row] - 1 ...
#else
... mNPads[row] ...

Will you change this PR, or do you want me to create one?

[shahor02]

@davidrohr OK, I can change this PR by the next Wednesday.

[shahor02]

@davidrohr please check. Note that the GPUTPCCompressionTrackModel has no GPUParam data member when neither GPUCA_COMPRESSION_TRACK_MODEL_MERGER nor GPUCA_COMPRESSION_TRACK_MODEL_SLICETRACKER is defined, and this is the mode when LinearPad2Y and LinearY2Pad are used in the compression.
So, I had to change the signature of these methods, passing the npads and padwidth as arguments.

[alibuild]

Error while checking build/O2/fullCI_slc9 for cf1958a at 2025-01-22 17:24:

## sw/BUILD/ONNXRuntime-latest/log
CMake Error at /sw/slc9_x86-64/CMake/v3.28.1-13/share/cmake-3.28/Modules/CMakeFindDependencyMacro.cmake:76 (find_package):

Full log here.

[davidrohr]

looks good, thx!