-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Parallel overlap checking with staged workflow #20963
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.
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
base: master
Are you sure you want to change the base?
Conversation
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
|
Working on fixing no-IMT mode |
Test Results 22 files 22 suites 3d 14h 56m 12s ⏱️ For more details on these failures, see this check. Results for commit 793c33b. ♻️ 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
|
The failure in CI is on Windows only, it seems to throw an exception in TThreadExecutor::Foreach during overlap checking, not because the test gtest-geom-boolean-extrusion produces a wrong result, because I added a print that would appear in case of a bad result and it doesn't. I don't know if this is a known feature on Windows. The PR is now ready for review. |
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:
TGeoManager::SetPoints()(default 20) andTGeoManager::SetNmeshVertices()(default 1000).TGeoChecker.TBuffer3DbuffersTGeoCompositeShape / TGeoBoolNodeGetPointsOnSegments()contracts in several shapesParallel execution
ROOT::TThreadExecutorand respectsROOT::EnableImplicitMT(N).Correctness fixes in meshing
GetPointsOnSegments()for several shapes to:trueonly if exactlynpointsare filledTGeoBBox,TGeoTube,TGeoCone,TGeoPcon,TGeoCtub).Performance impact
Checklist:
This PR was done at the request of ALICE experiment, but provides setup-independent enhancements of the overlap checking feature