Skip to content

faster mat LUT - #15712

Open
f3sch wants to merge 1 commit into
AliceO2Group:devfrom
f3sch:matlut/opt
Open

faster mat LUT#15712
f3sch wants to merge 1 commit into
AliceO2Group:devfrom
f3sch:matlut/opt

Conversation

@f3sch

@f3sch f3sch commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Halve the layer voxel lookup table by storing only the lower interval per voxel and moving the "voxel on a boundary" information into the entry's top bit (40 -> 20 kB). NOTE that this causes all current matlut.dump to be invalidated.
The upper candidate is recomputed in resolveLayerRange() for the few percent of ambiguous voxels (5%), using arithmetic identical to initLayerVoxelLU() so bracketing is unchanged.
With one entry per radius, getLayersRange() can issue the min and max lookups as two independent loads before resolving either.
On the device the voxel table always exists, so the mInitializedLayerVoxelLU check is compiled out.
In getMatBudget(), replace the per-cell modulo by a compare (phiID is at most one revolution off) and hoist the phi row base pointer out of the z loop via the new MatLayerCyl::getCellRow().
Fix mR2Intervals[1], which kept the global rMax2 instead of layer 0's rMax2 while the fill loop starts at index 2.
The array was therefore non-monotonic for any multi-layer LUT, making searchSegment() results depend on the bracket passed in, and the gap-detection loop reads that same entry at i == 1, so a gap between the first two layers could never be registered.
This especially happens in ITS since the mRIntervals is currently [0, 250000, 3.6864, 4.84, ...] which always causes whole linear scan in for the region between 0-3.2 cm
This will only apply for LUTs that are rebuilt.

For the ITS kernels this results in 5% faster lookup.

scope before after delta
LUT-using kernels 4578 ms 4347 ms −5.0%
Track finding stage 6.47 s 6.17 s −4.6%
tracking total 11.05 s 10.75 s −2.7%
workflow total 16.37 s 16.08 s −1.8%

f3sch added a commit to f3sch/AliceO2 that referenced this pull request Aug 24, 2026
Please consider the following formatting changes to AliceO2Group#15712
@f3sch
f3sch marked this pull request as ready for review August 24, 2026 10:50

@shahor02 shahor02 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.

Thanks, looks good, see a couple of questions below.
Since the PR changes the LUT layout, we either have to switch to a new CCDB path for new objects, or, to reformat the old objects to new layout in the rectifyPtrFromFile

Comment thread Detectors/Base/src/MatLayerCylSet.cxx
Comment thread Detectors/Base/src/MatLayerCylSet.cxx Outdated
@f3sch

f3sch commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

Since the PR changes the LUT layout, we either have to switch to a new CCDB path for new objects, or, to reformat the old objects to new layout in the rectifyPtrFromFile

The persisted layout does not change, right? mLayerVoxelLU is //! transient and this FlatObject buffer (estimateFlatBufferSize()) is rebuilt by initLayerVoxelLU() on every load.

The only change that should go into the serialisation is mR2Intervals[1] but I think it is fine to wait for us to rebuilt the LUT to fix this.

f3sch added a commit to f3sch/AliceO2 that referenced this pull request Aug 24, 2026
Please consider the following formatting changes to AliceO2Group#15712
@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2/fullCI_slc9 for 222fb19 at 2026-08-24 17:00:

## sw/BUILD/O2-latest/log
/sw/SOURCES/O2/slc9_x86-64-slc9_x86-64/0/Detectors/Base/src/MatLayerCylSet.cxx:605:62: error: expected ',' before '-' token
/sw/SOURCES/O2/slc9_x86-64-slc9_x86-64/0/Detectors/Base/src/MatLayerCylSet.cxx:605:63: error: expected identifier before '-' token
/sw/SOURCES/O2/slc9_x86-64-slc9_x86-64/0/Detectors/Base/src/MatLayerCylSet.cxx:605:67: error: expected '{' before ';' token
/sw/SOURCES/O2/slc9_x86-64-slc9_x86-64/0/Detectors/Base/src/MatLayerCylSet.cxx:605:30: error: operands to '?:' have different types 'int' and 'o2::base::MatLayerCylSet::getLayersRange(const o2::base::Ray&, short int&, short int&) const::<lambda()>'
ninja: build stopped: subcommand failed.

Full log here.

shahor02
shahor02 previously approved these changes Aug 24, 2026

@shahor02 shahor02 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.

Indeed, forgot that the mLayerVoxelLU is filled dynamically.

@f3sch

f3sch commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

@shahor02 sorry, blindly copied your suggestion for [NumVoxels-1] (should have been NumVoxels-1)

Comment thread Detectors/Base/src/MatLayerCylSet.cxx Outdated
Comment thread Detectors/Base/src/MatLayerCylSet.cxx Outdated
Comment thread Detectors/Base/src/MatLayerCylSet.cxx
Comment thread Detectors/Base/include/DetectorsBase/MatLayerCylSet.h
@sawenzel

Copy link
Copy Markdown
Collaborator

Consider two assertions in testMatBudLUT, because nothing there touches this path today — testMBLUT() only diffs print(true) across build/read/clone/relocate. Checking that mR2Intervals is monotonic after finalizeStructures(), and that getLayersRange() agrees between the mInitializedLayerVoxelLU false and true paths over a few thousand random rays, would cover both changes.

Signed-off-by: Felix Schlepper <felix.schlepper@cern.ch>
@f3sch

f3sch commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

Consider two assertions in testMatBudLUT, because nothing there touches this path today — testMBLUT() only diffs print(true) across build/read/clone/relocate. Checking that mR2Intervals is monotonic after finalizeStructures(), and that getLayersRange() agrees between the mInitializedLayerVoxelLU false and true paths over a few thousand random rays, would cover both changes.

Ok done. Note that this does not enforces that current already produced LUTs are monotonic since this would cause to invalidate all objects

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.

4 participants