Parallel overlap checking with staged workflow by agheata · Pull Request #20963 · root-project/root

agheata · 2026-01-20T12:19:19Z

This Pull request:

Refactors and accelerates geometry overlap checking by splitting the procedure into well-defined stages, introducing candidate filtering and parallel execution, and fixing long-standing meshing and caching issues affecting correctness and performance. Preserves backward-compatibility of interfaces.

Changes or fixes:

Three-stage overlap checking workflow

Candidate enumeration
- Traverses the geometry tree.
- Uses oriented bounding box (OBB) tests to reject non-overlapping volume pairs early. This gives in general a sizeable candidate reduction compared to the legacy version, and a massive one for assembly-dominated setups (a factor of ~1000 for ALICE O² geometry)
Surface point generation and caching
- Generates surface sampling points per shape using the values set via TGeoManager::SetPoints() (default 20) and TGeoManager::SetNmeshVertices() (default 1000).
- Introduces a thread-safe per-shape mesh cache inside TGeoChecker.
- Cache invalidation is correctly handled when the number of mesh points or mesh segments changes
- Fixes incorrect behavior caused by:
  - reuse of static TBuffer3D buffers
  - stale mesh caches in TGeoCompositeShape / TGeoBoolNode
  - inconsistent GetPointsOnSegments() contracts in several shapes
Exact overlap checking following check points density
- Performs navigation-based overlap tests using cached surface points.
- This stage is now parallelized using ROOT Implicit MT.

Parallel execution
- Only the final overlap checking stage is parallelized (safe and scalable).
- Uses ROOT::TThreadExecutor and respects ROOT::EnableImplicitMT(N).
- Demonstrates good strong scaling on realistic detector geometries.
Correctness fixes in meshing
- Reworked GetPointsOnSegments() for several shapes to:
  - avoid dependence on TBuffer3D static storage
  - generate points only on valid surface generators
  - respect a strict contract: return true only if exactly npoints are filled
- Fixed incorrect assumptions in legacy implementations (e.g. TGeoBBox, TGeoTube, TGeoCone, TGeoPcon, TGeoCtub).
Performance impact
- The check is more thorough (points more uniformly spread on solid surfaces) and typically much faster, because it reduces the number of candidates for the expensive check, and parallelizes this most expensive part.
- On typical setups (e.g. cmsRun4D116), the gain using 32 threads is significant

On assembly-rich setups (e.g. o2sim_geometry) the gain is massive (from ~4 hours to ~15 sec on the same machine)
Below is a strong scaling plot showing both the total time and the time for the parallelized checking stage for ALICE. Note that this is not the typical profile, it is shown mainly to show the scaling of the parallelized stage. For most setups, the weight of the sequential stages 1 and 2 is much smaller that the weight of the parallel stage 3.

Checklist:

tested changes locally
updated the docs (if necessary)

This PR was done at the request of ALICE experiment, but provides setup-independent enhancements of the overlap checking feature

Voxels marked as needing rebuild are built lazily, but this is a thread unsafe operation as it touches shared data. This introduces a method TGeoManager::RebuildVoxels that is called before checking overlaps, removing rebuilding potentially happening during the checking. The method TGeoChecker::CheckOverlaps is rewritten to avoid changing the list of overlaps attached to nodes, which was also a thread unsafe construct.

…;lelizable compute phase. Checking overlaps by sampling points in volumes implemented now as CheckOverlapsBySampling, and marked as deprecated. In future we will support a surface-based sampling like in Geant4.

…sing the separation axis theorem (SAT).

…y checks. This reduces by a large factor the number of real checks to be done. In assembly-based setups such as ALICE, the factor is ~1000.

…d added new ones. The legacy implementations for solids did not honour the contract that in case a shape overrides this method, it has to provide exactly the requested number of points if it returns true. The legacy versions even wrote more than the requested points, overwriting memory. Fixes cache overriding of points by visualization, that made the point generation not deterministic.

…site shapes. Previously, mesh points set via TGeoCompositeShape::SetPoints were cached only at the first call, making mesh generation invariant to the number of segments requested via TGeoManager. Now the caches are invalidated and rebuild lazily whenever the number of segment changes in the manager.

…cker implementation file

agheata · 2026-01-20T13:28:40Z

Working on fixing no-IMT mode

github-actions · 2026-01-20T18:59:51Z

Test Results

22 files 22 suites 3d 15h 36m 6s ⏱️
3 771 tests 3 771 ✅ 0 💤 0 ❌
75 898 runs 75 898 ✅ 0 💤 0 ❌

Results for commit cf4af72.

♻️ This comment has been updated with latest results.

This was needed because the test is run in the same process as test_material_units, which changes the statically-initialized unit system in TGeoManager, inducing wrong material interpretation in GDML import used in the current test. This is not normal and should be debugged separately, but forcing the ROOT units seems to fix the problem Split the geometry tests to avoid static interference

geom/geom/inc/TGeoManager.h

geom/geom/inc/TGeoMatrix.h

geom/geom/inc/TGeoTube.h

geom/geom/inc/TVirtualGeoChecker.h

dpiparo · 2026-01-28T07:51:32Z

geom/geom/src/TGeoBBox.cxx

-         ipoints--;
+         nperseg = remaining;
+
+      const Double_t dx = (p1[0] - p0[0]) / (nperseg + 1);


if this is performance critical, we could avoid 3 divisions and just have one

It is not, but I agree it can be optimized

geom/geom/src/TGeoBBox.cxx

dpiparo · 2026-01-28T07:54:22Z

geom/geom/src/TGeoCone.cxx

+            continue;
+
+         const Double_t phi = ig * dphi;
+         const Double_t c = TMath::Cos(phi);


opportunity to save cycles with sincos.

Yes, there are many places where this can be done

I wonder why sincos is not available in TMath?

dpiparo · 2026-01-28T07:55:50Z

geom/geom/src/TGeoCone.cxx

+
+         // Center of the φ bin => never exactly on the cut planes
+         const Double_t phi = phi1 + (ig + 0.5) * dphi;
+         const Double_t c = TMath::Cos(phi);


sincos opportunity

geom/geom/src/TGeoMatrix.cxx

geom/geom/src/TGeoNode.cxx

geom/geom/src/TGeoPcon.cxx

dpiparo · 2026-01-28T08:01:35Z

geom/geom/src/TGeoPcon.cxx

+      if (dz == 0.0)
+         continue;
+      const Double_t len = std::abs(dz);
+      zsegs.push_back({i, z0, z1, len});


Agreed in general for complex/heavy constructors, but nowadays for trivial structs push_back and emplace_back generate the same code, the compiler moves the data from the temporary to the vector memory (POD copy), inlining and eliding aggressively, so no temporaries survive codegen

dpiparo · 2026-01-28T08:02:14Z

geom/geom/src/TGeoPcon.cxx

+
+            // Generator center angle: avoids phi endpoints
+            const Double_t phi = (phi1Deg + (ig + 0.5) * dphi) * TMath::DegToRad();
+            const Double_t c = TMath::Cos(phi);


sincos opportunity?

dpiparo · 2026-01-28T08:03:24Z

geom/geom/src/TGeoTube.cxx

+   Int_t outPoints = npoints;
+   Int_t inPoints = 0;
+   if (hasInner) {
+      outPoints = (npoints + 1) / 2;


opportunity for one division less?

you mean: outPoints = (npoints + 1) >> 1 ? If yes, I was doing this agressively at some point, until I discovered that it is worse: it hides the intention (looks more like a hack, not math), on signed integers it is wrong, and the compiler eventually generates the same code. So n / 2 lets the compiler take the decision on the implementation

dpiparo · 2026-01-28T08:03:57Z

geom/geom/src/TGeoTube.cxx

+      const Int_t rem = nSurfPoints % nGen;
+
+      // Use generator centers: never exactly phi1 or phi2
+      const Double_t dphi = dPhiTot / (Double_t)nGen;


no need to cast to double

Agreed, I'm a bit maniac due to the work on mixed precision in VecGeom. Harmless here.

dpiparo · 2026-01-28T08:04:03Z

geom/geom/src/TGeoTube.cxx

+            continue;
+
+         const Double_t phi = phi1 + (ig + 0.5) * dphi; // avoid endpoints
+         const Double_t c = TMath::Cos(phi);


The problem is that sincos is a non-standard extension that exists on most platforms, but not on all (such as mac/windows flavors), so IMHO TMath should (it does not) provide a safe wrapper for it that falls back on regilar Sin/Cos where not available). Or maybe Iam wrong?

dpiparo · 2026-01-28T08:04:29Z

geom/geom/src/TGeoTube.cxx

+
+         const Double_t phi = phi1 + (ig + 0.5) * dphi; // avoid endpoints
+         const Double_t c = TMath::Cos(phi);
+         const Double_t s = TMath::Sin(phi);


dpiparo · 2026-01-28T08:04:53Z

geom/geom/src/TGeoVolume.cxx

-#include "TMap.h"
-#include "TFile.h"
-#include "TKey.h"
+#include <TString.h>


stl and root's include order

geom/geom/src/TGeoVolume.cxx

dpiparo · 2026-01-28T08:05:47Z

geom/geom/src/TGeoVolume.cxx

 void TGeoVolume::SetFWExtension(TGeoExtension *ext)
 {
-   TGeoExtension* tmp = fFWExtension;
+   TGeoExtension *tmp = fFWExtension;


use of this line?

dpiparo · 2026-01-28T08:06:24Z

geom/geomchecker/inc/TGeoChecker.h

 #define ROOT_TGeoChecker

 #include "TVirtualGeoChecker.h"
+#include "TGeoOverlapCandidate.h"


stl - ROOT includes order

geom/geomchecker/src/TGeoChecker.cxx

dpiparo · 2026-01-28T08:09:50Z

geom/geomchecker/src/TGeoChecker.cxx

-   UInt_t id;
-   Int_t level;
-   // Check extrusion only for daughters of a non-assembly volume
+   Int_t ncand = 0;


please solve these warnings

Did I miss some warnings?

dpiparo · 2026-01-28T08:11:04Z

geom/test/test_boolean_extrusion.cxx

@@ -1,6 +1,7 @@
 #include <gtest/gtest.h>


is this test enough? Maybe there are more cases to check and that would be very useful!

Indeed, we can have a more extended check, but the thing with overlap checking is that the results may change if for some reason, the points generated on the shape meshes change. Overlap checking only strives not to give false positives, but may miss. So this case is exactly doing this: knows that the shapes mother/daughter have some common faces, but this is correct, so no extrusion should be reported. Actually this unit test triggered the error on Windows, that was a real bug, and also other errors when changing some global static and having 2 geometry managers in the same process.

agheata and others added 15 commits January 20, 2026 10:42

[geom] Legacy overlap checker now independent wrt checking order.

20aeacd

[geom] Remove typos in CheckGeometry.

6107f8f

[geom] Correction for TGeoChecker::SamplePoints.

5a9badc

[geom] Overlap checking now split in an enumeration phase and a paral…

54235b9

…;lelizable compute phase. Checking overlaps by sampling points in volumes implemented now as CheckOverlapsBySampling, and marked as deprecated. In future we will support a surface-based sampling like in Geant4.

[geom] Implemented IsConvex checks for all shapes.

422c3e5

[geom] Implemented general oriented bounding box (OBB) intersection u…

ce4989e

…sing the separation axis theorem (SAT).

[geom] Expose candidate counters from EnumerateOverlapCandidates.

7863a80

[geom] Activated OBB checks in overlap checking, including in assembl…

203b1e4

…y checks. This reduces by a large factor the number of real checks to be done. In assembly-based setups such as ALICE, the factor is ~1000.

[geom] First implementation of parallelism in CheckOverlaps.

e1dafd9

[geom] Minor printout adjustments.

cbeb3af

[geom] Documented CheckOverlaps similar to other utilities in TGeoChe…

04943d9

…cker implementation file

[geom] Added explicit dependency of Geom on Imt.

9815ba7

agheata self-assigned this Jan 20, 2026

agheata requested a review from bellenot as a code owner January 20, 2026 12:19

agheata added this to Fixed in: not applicable Jan 20, 2026

agheata added new feature improvement priority:default in:Geometry labels Jan 20, 2026

[geom] Added missing TGeoOverlapCandidate.h

b37881f

[geom] Protect parallel checking for no IMT available.

03e59a5

agheata marked this pull request as draft January 22, 2026 09:38

agheata force-pushed the par_ovlp branch from 608785b to d828cb6 Compare January 22, 2026 12:59

[geom] Print failure trace for the extrusion test.

793c33b