Skip to content

ITS: new CPU + GPU seeding vertexer - #15733

Open
cima22 wants to merge 1 commit into
AliceO2Group:devfrom
cima22:parallel-vertexer
Open

cima22 wants to merge 1 commit into
AliceO2Group:devfrom
cima22:parallel-vertexer

Conversation

@cima22

@cima22 cima22 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Adds an optional seeding vertexer that runs as a prepended tracker pass (diamond trackleting -> cells -> lines -> parallel seeding), on both the CPU and GPU traits.
Additionally, unsorted clusters are loaded to GPU and then sorted on GPU.

To set it:
ITSCATrackerParam.seedingVertexIteration=1;
ITSVertexerParam.useParallelSeeding=1;

Tuned parameters for Pb--Pb:

ITSCATrackerParam.seedingVertexIteration=1;ITSVertexerParam.useParallelSeeding=1;ITSVertexerParam.useTruthSeeding=0;ITSCATrackerParam.diamondTrackletingPVres=1.5102214480735774;ITSCATrackerParam.diamondTrackletingNSigmaCut=2.976946815605669;ITSCATrackerParam.diamondTrackletingCellDeltaTanLambdaSigma=0.005566295924057422;ITSCATrackerParam.diamondCellTanLambdaNSigma=2.5;ITSCATrackerParam.diamondTrackletingCellDeltaPhiMinPt=0.12;ITSCATrackerParam.cellLineSharedClusterCut=1;ITSVertexerParam.clusterCut=0.07;ITSVertexerParam.pairCut=0.025684202919354766;ITSVertexerParam.phiCut=0.008875191490279988;ITSVertexerParam.clusterContributorsCut=3;ITSVertexerParam.lineMinPt=0.10;ITSVertexerParam.nSigmaCut=0.0016;ITSVertexerParam.goodLineChi2Cut=9.814679066181695;ITSVertexerParam.goodLinePtCut=0.12;ITSVertexerParam.goodContributorsSignificance=0.070;ITSVertexerParam.suppressLowMultDebris=13;ITSVertexerParam.fineZWindow=0.010;ITSVertexerParam.fineMinDensity=8;ITSVertexerParam.fineMaxDrift=0.005;ITSVertexerParam.duplicateZScale=0.7;ITSVertexerParam.duplicateZCut=0.1388966993405415;

Tuned parameters for pp:

ITSCATrackerParam.seedingVertexIteration=1;ITSVertexerParam.useParallelSeeding=1;ITSVertexerParam.useTruthSeeding=0;ITSCATrackerParam.diamondTrackletingPVres=3.74344532796542;ITSCATrackerParam.diamondTrackletingNSigmaCut=5.352263543811316;ITSCATrackerParam.diamondTrackletingCellDeltaTanLambdaSigma=0.002563403956702525;ITSCATrackerParam.diamondCellTanLambdaNSigma=3.257473162361808;ITSCATrackerParam.diamondTrackletingCellDeltaPhiMinPt=0.09407482537938365;ITSCATrackerParam.cellLineSharedClusterCut=3;ITSVertexerParam.clusterCut=0.05557266167329535;ITSVertexerParam.pairCut=0.05176031785438137;ITSVertexerParam.phiCut=0.017639175978851104;ITSVertexerParam.clusterContributorsCut=2;ITSVertexerParam.lineMinPt=0.1;ITSVertexerParam.nSigmaCut=0.0016;ITSVertexerParam.goodLineChi2Cut=9.814679066181695;ITSVertexerParam.goodLinePtCut=0.12;ITSVertexerParam.goodContributorsSignificance=0.07;ITSVertexerParam.suppressLowMultDebris=13;ITSVertexerParam.fineZWindow=0.013014502308220061;ITSVertexerParam.fineMinDensity=4;ITSVertexerParam.fineMaxDrift=0.004003953018135234;ITSVertexerParam.duplicateZScale=0.1230148343342053;ITSVertexerParam.duplicateZCut=0.06571512005272727;

@shahor02

Copy link
Copy Markdown
Collaborator

Hi @cima22 , are there some slides or plots on the new algo., particularly its performance? Perhaps you could present it next Wed., either at the ITS meeting at 11 (if it is planned, @fprino ?) or at sim/rec WP12/13 at 15:30?

@f3sch f3sch left a comment

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.

Hi @cima22, thanks! Some preliminary comments although I have to admit to not understand the logic in its entirety yet.

Comment thread Detectors/ITSMFT/ITS/tracking/GPU/cuda/TimeFrameGPU.cu

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.

Do we need to duplicate the already existing ClusterLines math, can we not have only one of these classes?

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.

Could we promote the GPU class to be the common one? The CPU one uses ROOT, I don't think it is available for GPUs?

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.

sure, you can adapt the CPU class however you like to make it GPU compatible.

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.

now there should be only one class 👍

Comment thread Detectors/ITSMFT/ITS/tracking/GPU/ITStrackingGPU/TimeFrameGPU.h Outdated
Comment thread Detectors/ITSMFT/ITS/tracking/GPU/ITStrackingGPU/TimeFrameGPU.h Outdated
Comment thread Detectors/ITSMFT/ITS/tracking/GPU/ITStrackingGPU/TimeFrameGPU.h Outdated
Comment thread Detectors/ITSMFT/ITS/tracking/include/ITStracking/TrackingConfigParam.h Outdated
Comment thread Detectors/ITSMFT/ITS/tracking/include/ITStracking/TrackingConfigParam.h Outdated
Comment thread Detectors/ITSMFT/ITS/tracking/src/TrackerTraits.cxx Outdated
Comment thread Detectors/ITSMFT/ITS/tracking/src/TrackerTraits.cxx
Comment thread Detectors/ITSMFT/ITS/tracking/GPU/cuda/TrackingKernels.cu Outdated
@cima22

cima22 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

@shahor02 This algorithm should be tested, but I have some preliminary results.

On 50 simulated PbPb TFs at 50kHz:

efficiency_vs_mult mult_fake_vs_matched z_residuals

On a PbPb TF with embedded pp collisions:

PbPb:

efficiency_vs_mult

pp:

efficiency_vs_mult

pp track efficiency:

efficiency_vs_pt

@shahor02

Copy link
Copy Markdown
Collaborator

Thanks, looks good! Note that there are conflicts in the PR.
So, will you be available to present it next Wed.?

@cima22

cima22 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, looks good! Note that there are conflicts in the PR. So, will you be available to present it next Wed.?

Yes absolutely

@fprino

fprino commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks, looks good! Note that there are conflicts in the PR. So, will you be available to present it next Wed.?

Yes absolutely

Sorry for replying late: next week we are considering to move the ITS WP2 meeting from Wednesday 2 september to Thursday 3 September, starting at 10 am.
Would this work for you (Gabriele, Ruben, Felix)?

@f3sch

f3sch commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

@fprino I will be on vacation from Thursday onwards.

@f3sch

f3sch commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Some things from an initial test:
Timing:

[PROD][fschlepp@epn000 pvtest]$ python3 time.py gabriele/reco.log
Processing:
Original count: 925
Trimmed count:  649
Discarded:      138 low + 138 high
Total:          336405.15 ms (336.41 s)
Average:        518.34 ms
Std deviation:  194.22 ms

Vertex seeding:
Original count: 925
Trimmed count:  649
Discarded:      138 low + 138 high
Total:          60625.25 ms (60.63 s)
Average:        93.41 ms
Std deviation:  13.94 ms

Tracking:
Original count: 919
Trimmed count:  645
Discarded:      137 low + 137 high
Total:          231575.46 ms (231.58 s)
Average:        359.03 ms
Std deviation:  86.99 ms
[PROD][fschlepp@epn000 pvtest]$ python3 time.py dev/reco.log

Processing:
Original count: 925
Trimmed count:  649
Discarded:      138 low + 138 high
Total:          511572.46 ms (511.57 s)
Average:        788.25 ms
Std deviation:  239.93 ms

Vertex seeding:
Original count: 925
Trimmed count:  649
Discarded:      138 low + 138 high
Total:          222618.46 ms (222.62 s)
Average:        343.02 ms
Std deviation:  89.72 ms

Tracking:
Original count: 924
Trimmed count:  648
Discarded:      138 low + 138 high
Total:          240011.05 ms (240.01 s)
Average:        370.39 ms
Std deviation:  70.58 ms

note this timing from MI50 EPNs. the new vertexing seems to be 3.7 times faster which in total means a speedup of 1.5 in total processing time (-34.2%).

23 kHz Pb-Pb.
red:dev, blue:this PR, green:this PR but relaxing ITSVertexerParam.clusterContributorsCut from 3 to 2:
pv_cont.pdf
There seems be recovery of PVs below 80 contributors and a slight degradation for higher mult vertices.

black:dev, red:this PR, blue:this PR but relaxing ITSVertexerParam.clusterContributorsCut from 3 to 2:
K0sFitOverlay.pdf
K0sFitVsPt.pdf
K0s yield decreases by 0.2% although this is dominated by the 1-1.5 GeV/c bin.

Note that there is currently no second iteration for UPC flagging.

@shahor02

Copy link
Copy Markdown
Collaborator

Then let's have it presented at the wp12/13 meeting next Wed. 15:30.
@cima22 Was suppressLowMultDebris=13 added to pp settings intentionally? Usually, we don't remove debris there.
@f3sch thanks, looks good. I assume for the UPC you left default settings?

@f3sch

f3sch commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

@f3sch thanks, looks good. I assume for the UPC you left default settings?

@shahor02 there is only one iteration done in this PR, if you mean the one from dev then yes.

@shahor02

Copy link
Copy Markdown
Collaborator

OK, I saw https://github.com/AliceO2Group/AliceO2/pull/15733/changes#diff-27fc8aaeb7769a496f838182d30a759ba63b3163bc7f458dd9bbbc895d3c7211R258-R260, but then the ITSVertexerParam are irrelevant in that mode, and there is no ITS trackfinding UPC iteration?
If the answer is long, don't waste time, will learn at the presentation.

@f3sch

f3sch commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

OK, I saw https://github.com/AliceO2Group/AliceO2/pull/15733/changes#diff-27fc8aaeb7769a496f838182d30a759ba63b3163bc7f458dd9bbbc895d3c7211R258-R260, but then the ITSVertexerParam are irrelevant in that mode, and there is no ITS trackfinding UPC iteration?

There is no track finding since no ROFs are flagged by a vertex carrying the UPC flag in the UPC mask, all ROFs are automatically masked in UPC iteration

@cima22

cima22 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Then let's have it presented at the wp12/13 meeting next Wed. 15:30.

@cima22 Was suppressLowMultDebris=13 added to pp settings intentionally? Usually, we don't remove debris there.

@f3sch thanks, looks good. I assume for the UPC you left default settings?

Hi, sorry today I am travelling so I won't be very responsive. I didn't put yet the UPC iteration in the PR, I will test one in the next days.

suppressLowMultDebris=13 slipped when copying the parameters, but in any case it is not used by this algorithm by default, thanks for noticing.

@cima22

cima22 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Fixed comments by @f3sch and added UPC iteration. For PbPb:

ITSCATrackerParam.diamondTrackletingCellDeltaPhiMinPt=0.12;ITSCATrackerParam.diamondCellTanLambdaNSigma=2.5;ITSCATrackerParam.seedingVertexIteration=1;ITSCATrackerParam.doUPCIteration=1;ITSCATrackerParam.cellLineSharedClusterCut=1;ITSCATrackerParam.diamondTrackletingPVres=1.5102214480735774;ITSCATrackerParam.diamondTrackletingNSigmaCut=2.976946815605669;ITSCATrackerParam.diamondTrackletingCellDeltaTanLambdaSigma=0.005566295924057422;ITSVertexerParam.suppressLowMultDebris=13;ITSVertexerParam.goodLineChi2Cut=9.814679066181695;ITSVertexerParam.goodLinePtCut=0.12;ITSVertexerParam.clusterContributorsCut=3;ITSVertexerParam.clusterCut=0.07;ITSVertexerParam.duplicateZCut=0.1388966993405415;ITSVertexerParam.pairCut=0.025684202919354766;ITSVertexerParam.nSigmaCut=0.0016;ITSVertexerParam.phiCut=0.008875191490279988;ITSVertexerParam.goodContributorsSignificance=0.070;ITSVertexerParam.fineZWindow=0.010;ITSVertexerParam.fineMinDensity=8;ITSVertexerParam.fineMaxDrift=0.005;ITSVertexerParam.lineMinPt=0.10;ITSVertexerParam.duplicateZScale=0.7;

For PbPb UPC iteration, clusterContributorsCut is set to 2, but it may need other tuning.

@cima22
cima22 requested a review from a team as a code owner September 4, 2026 13:25
@alibuild

alibuild commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2/fullCI_slc9 for 6c20ad5 at 2026-09-05 15:28:

## sw/BUILD/O2Physics-latest/log
c++: fatal error: Killed signal terminated program cc1plus
c++: fatal error: Killed signal terminated program cc1plus


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
grep: error-log.txt: binary file matches
++ GRERR=1
++ [[ 1 == 0 ]]
++ mkdir -p /sw/INSTALLROOT/2820ef2fd7971b0b882b12016412662dd544f397/slc9_x86-64/o2checkcode/1.0-local557/etc/modulefiles
++ alibuild-generate-module
+ HAS_BIN=
+ HAS_LIB=
+ HAS_CMAKE=
+ HAS_ROOT=
+ '[' 0 '!=' 0 ']'
+ cat
+ printf 'if ![ is-loaded '\''BASE/1.0'\'' ] {\n module load BASE/1.0\n}'
+ echo 'BUILD_REQUIRES=CMake alibuild-recipe-tools defaults-release'
BUILD_REQUIRES=CMake alibuild-recipe-tools defaults-release
+ FULL_BUILD_REQUIRES='FairCMakeModules safe_int UUID googlebenchmark double-conversion ninja-fortran Python-modules-list defaults-release Alice-GRID-Utils MPFR json-c date bz2 ninja pytorch_cpuinfo O2-customization alibuild-recipe-tools CMake'
+ echo 'FULL_BUILD_REQUIRES=FairCMakeModules safe_int UUID googlebenchmark double-conversion ninja-fortran Python-modules-list defaults-release Alice-GRID-Utils MPFR json-c date bz2 ninja pytorch_cpuinfo O2-customization alibuild-recipe-tools CMake'
FULL_BUILD_REQUIRES=FairCMakeModules safe_int UUID googlebenchmark double-conversion ninja-fortran Python-modules-list defaults-release Alice-GRID-Utils MPFR json-c date bz2 ninja pytorch_cpuinfo O2-customization alibuild-recipe-tools CMake
++ env
++ cut -f1 -d=
++ grep -v '^DEFAULT_'
++ grep -v PKGREVISION
++ grep REVISION
++ grep -v ALIBUILD_RECIPE_TOOLS
++ sed -e s/_REVISION//
+ for x in $(env | cut -f1 -d= | grep -v "^DEFAULT_" | grep -v PKGREVISION | grep -v ALIBUILD_RECIPE_TOOLS | grep REVISION | sed -e 's/_REVISION//')
++ eval 'echo $LIBFFI_REVISION'
+++ echo 14
+ REVISION_VALUE=14
++ eval 'echo $LIBFFI_VERSION'
+++ echo v3.2.1-alice1
+ VERSION_VALUE=v3.2.1-alice1
++ eval 'echo $LIBFFI_ROOT'
+++ echo /sw/slc9_x86-64/libffi/v3.2.1-alice1-14
+ ROOT_PATH_VALUE=/sw/slc9_x86-64/libffi/v3.2.1-alice1-14
+ '[' -z 14 ']'
+ echo 'FairCMakeModules safe_int UUID googlebenchmark double-conversion ninja-fortran Python-modules-list defaults-release Alice-GRID-Utils MPFR json-c date bz2 ninja pytorch_cpuinfo O2-customization alibuild-recipe-tools CMake'
+ tr '[:lower:]' '[:upper:]'
+ tr - _
[0 more errors; see full log]

Full log here.

Adds a seeding vertexer that runs as a prepended tracker pass (diamond
trackleting -> cells -> lines -> parallel seeding), on both the CPU and
GPU traits, replacing the per-ROF CPU vertexer for the seeding step.
@alibuild

alibuild commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2/fullCI_slc9 for 9257bc9 at 2026-09-07 19:11:

## sw/BUILD/O2-full-system-test-latest/log
command alien-token-init had nonzero exit code 2


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
grep: error-log.txt: binary file matches
++ GRERR=1
++ [[ 1 == 0 ]]
++ mkdir -p /sw/INSTALLROOT/669a1b23d87054378167507b6800fd1385f5d456/slc9_x86-64/o2checkcode/1.0-local2/etc/modulefiles
++ alibuild-generate-module
+ HAS_BIN=
+ HAS_LIB=
+ HAS_CMAKE=
+ HAS_ROOT=
+ '[' 0 '!=' 0 ']'
+ cat
+ printf 'if ![ is-loaded '\''BASE/1.0'\'' ] {\n module load BASE/1.0\n}'
+ echo 'BUILD_REQUIRES=CMake alibuild-recipe-tools defaults-release'
BUILD_REQUIRES=CMake alibuild-recipe-tools defaults-release
+ FULL_BUILD_REQUIRES='ninja-fortran pytorch_cpuinfo FairCMakeModules UUID json-c safe_int double-conversion ninja MPFR bz2 googlebenchmark defaults-release O2-customization Python-modules-list date CMake alibuild-recipe-tools Alice-GRID-Utils'
+ echo 'FULL_BUILD_REQUIRES=ninja-fortran pytorch_cpuinfo FairCMakeModules UUID json-c safe_int double-conversion ninja MPFR bz2 googlebenchmark defaults-release O2-customization Python-modules-list date CMake alibuild-recipe-tools Alice-GRID-Utils'
FULL_BUILD_REQUIRES=ninja-fortran pytorch_cpuinfo FairCMakeModules UUID json-c safe_int double-conversion ninja MPFR bz2 googlebenchmark defaults-release O2-customization Python-modules-list date CMake alibuild-recipe-tools Alice-GRID-Utils
++ env
++ cut -f1 -d=
++ grep -v '^DEFAULT_'
++ grep -v PKGREVISION
++ grep -v ALIBUILD_RECIPE_TOOLS
++ grep REVISION
++ sed -e s/_REVISION//
+ for x in $(env | cut -f1 -d= | grep -v "^DEFAULT_" | grep -v PKGREVISION | grep -v ALIBUILD_RECIPE_TOOLS | grep REVISION | sed -e 's/_REVISION//')
++ eval 'echo $LIBFFI_REVISION'
+++ echo 14
+ REVISION_VALUE=14
++ eval 'echo $LIBFFI_VERSION'
+++ echo v3.2.1-alice1
+ VERSION_VALUE=v3.2.1-alice1
++ eval 'echo $LIBFFI_ROOT'
+++ echo /sw/slc9_x86-64/libffi/v3.2.1-alice1-14
+ ROOT_PATH_VALUE=/sw/slc9_x86-64/libffi/v3.2.1-alice1-14
+ '[' -z 14 ']'
+ echo 'ninja-fortran pytorch_cpuinfo FairCMakeModules UUID json-c safe_int double-conversion ninja MPFR bz2 googlebenchmark defaults-release O2-customization Python-modules-list date CMake alibuild-recipe-tools Alice-GRID-Utils'
+ tr '[:lower:]' '[:upper:]'
+ tr - _
+ tr ' ' '\n'
[0 more errors; see full log]

Full log here.

@f3sch f3sch left a comment

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.

Hi @cima22, some more nits to be addressed but I think besides the MC label on GPU thingy, we can merge this after a small local test.

Comment on lines 57 to +64
auto trackParams = TrackingMode::getTrackingParameters(mMode);
auto vertParams = TrackingMode::getVertexingParameters(mMode);
overrideParameters(trackParams, vertParams);
for (auto& p : trackParams) {
if (p.PassFlags[IterationStep::SeedingVertexPass]) {
p.CreateArtefactLabels = mIsMC;
}
}

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.

We should not change the tracking parameters outside of the Configuration.cxx, maybe we can just propagate mIsMC into the get*Parameters functions?

Comment on lines +828 to +835
const float cellDeltaPhiCut =
mTrkParams[iteration].PassFlags[IterationStep::SeedingVertexPass]
? math_utils::cellDeltaPhiBound(mBz, mTrkParams[iteration].CellDeltaPhiMinPt,
mTrkParams[iteration].LayerRadii[firstLink.fromLayer],
mTrkParams[iteration].LayerRadii[firstLink.toLayer],
mTrkParams[iteration].LayerRadii[secondLink.toLayer],
mTimeFrame->getLinkMSAngle(cellTopology.firstLink))
: -1.f;

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.

nit: the indentation feels off here should it not be something like:

Suggested change
const float cellDeltaPhiCut =
mTrkParams[iteration].PassFlags[IterationStep::SeedingVertexPass]
? math_utils::cellDeltaPhiBound(mBz, mTrkParams[iteration].CellDeltaPhiMinPt,
mTrkParams[iteration].LayerRadii[firstLink.fromLayer],
mTrkParams[iteration].LayerRadii[firstLink.toLayer],
mTrkParams[iteration].LayerRadii[secondLink.toLayer],
mTimeFrame->getLinkMSAngle(cellTopology.firstLink))
: -1.f;
const float cellDeltaPhiCut = mTrkParams[iteration].PassFlags[IterationStep::SeedingVertexPass]
? math_utils::cellDeltaPhiBound(mBz, mTrkParams[iteration].CellDeltaPhiMinPt,
mTrkParams[iteration].LayerRadii[firstLink.fromLayer],
mTrkParams[iteration].LayerRadii[firstLink.toLayer],
mTrkParams[iteration].LayerRadii[secondLink.toLayer],
mTimeFrame->getLinkMSAngle(cellTopology.firstLink))
: -1.f;

Comment on lines -324 to 348

for (int iLayer{0}; iLayer < NLayers; ++iLayer) {
clearResizeBoundedVector(mIndexTables[iLayer], getNrof(iLayer) * ((trkParam.ZBins * trkParam.PhiBins) + 1), getMaybeFrameworkHostResource());
}
for (int iLayer{0}; iLayer < trkParam.NLayers; ++iLayer) {
if (trkParam.SystErrorY2[iLayer] > 0.f || trkParam.SystErrorZ2[iLayer] > 0.f) {
for (auto& tfInfo : mTrackingFrameInfo[iLayer]) {
/// Account for alignment systematics in the cluster covariance matrix
tfInfo.covarianceTrackingFrame[0] += trkParam.SystErrorY2[iLayer];
tfInfo.covarianceTrackingFrame[2] += trkParam.SystErrorZ2[iLayer];
allocateClusterSortStorage(trkParam, maxLayers);
// Applied by the only FirstPass pass that covers every layer (tracking iteration 0).
// The 3-layer seeding passes must be excluded: they run first and would otherwise apply this to layers 0..2 only
if (trkParam.NLayers == NLayers) {
for (int iLayer{0}; iLayer < trkParam.NLayers; ++iLayer) {
if (trkParam.SystErrorY2[iLayer] > 0.f || trkParam.SystErrorZ2[iLayer] > 0.f) {
for (auto& tfInfo : mTrackingFrameInfo[iLayer]) {
tfInfo.covarianceTrackingFrame[0] += trkParam.SystErrorY2[iLayer];
tfInfo.covarianceTrackingFrame[2] += trkParam.SystErrorZ2[iLayer];
}
}
}
}

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.

maybe I misunderstand, but why apply the additional uncertainty on the cluster position not for the seeding pass?

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.

I am not sure why this is necessary, the MC labels we only compute on the host (and general we are not using the labels in the first place for GPU)?

Comment on lines -30 to -34
#pragma link C++ class o2::its::Line + ;
#pragma link C++ class std::vector < o2::its::Line> + ;

#pragma link C++ class o2::its::ClusterLines + ;
#pragma link C++ class std::vector < o2::its::ClusterLines> + ;

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.

not that it is important since we never used it but why do we drop the dictionaries?

const bool useDiamond = this->mTrkParams[iteration].UseDiamond;
if (useDiamond) {
const Vertex diamondVert(this->mTrkParams[iteration].Diamond, this->mTrkParams[iteration].DiamondCov, 1, 1.f);
mTimeFrameGPU->createDiamondDevice(diamondVert);

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.

I would probably have increased simply the time error on the diamond to be max, called updateROFLookup once and avoided the extra gating on vtxMode

Comment on lines +94 to +99
total += evaluateTask(&Tracker::computeBeamFromVertices, StateNames[mCurStep = BeamPositioning], it, logger, it);
const float dx = mTimeFrame->getBeamX() - prevBeamX;
const float dy = mTimeFrame->getBeamY() - prevBeamY;
if (dx * dx + dy * dy < constants::BeamConvergence2) {
logger(std::format(" - Beam bootstrap converged after pass {} (beam shift < 50 um)", pass));
break;

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.

you have the early return in the computeBeamFromVertices but I would lift it to be here without running any of this.

ROFVertexLookupTableN::View mROFVertexLookupTableView;
ROFMaskTableN mMultiplicityCutMask;
ROFMaskTableN mUPCCutMask;
ROFMaskTableN mSeedingUPCMask;

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.

Why is there an extra mask required for the UPC seeding?

Comment on lines +341 to +347
std::vector<o2::its::Line> mLinesHost;
std::vector<int> mLineRofHost;
std::vector<int> mLineClustersHost;
std::vector<VertexCand> mVertexCandsHost;
std::vector<int> mPeakOffsetsHost;
PeakMembershipHost mPeakMembershipHost;
std::vector<o2::MCCompLabel> mLineLabelFlatHost;

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.

should these not use the bounded_vector?

Comment on lines +868 to +881
GPUdi() int clusterROF(const int* rofArr, const int nRofs, const int clusterIdx)
{
const int key = clusterIdx + 1;
int lo = 0, hi = nRofs + 1;
while (lo < hi) {
const int mid = (lo + hi) >> 1;
if (rofArr[mid] < key) {
lo = mid + 1;
} else {
hi = mid;
}
}
return lo - 1;
}

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.

debris, this should go into the Utils.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants