acts-project / acts-project/acts

Bug: Unpredicatable navigation in layer with overlapping surface

Open
#1,852 5 comments 0 reactions 0 assignees View on GitHub
Bug Component - Core Impact - Minor Stale
Dominant language
C++
Stars
131
Forks
276
Avg merge
3d 13h
Merged PRs (30d)
112

Description

While helping people perform the material mapping with a simple "testbeam like" detector, I notice an issue with the navigation. If in a layer we have 2 (or more) surfaces at the same position (ie : the `pathlenght` from the previous surface is the same) then the navigation will only visit one of those surfaces (which expected) but which one is not predictable.

In the sorting of the layer surfaces (in Layer.cpp line 231) we chain a sort based on the surface memory address with one based on` pathlenght` this mean for two surface with the same `pathlenght` it is impossible to predict which one will be first.

In the particular example I mention at the start the representing surface and the sensitive surface ended up being at the same position, this meant that when trying to map onto the sensitive surface half of them did have material (since the representing surface was crossed first).

I see three possible solutions to this issue :
- Return an error during the layer construction if multiple layer overlap (but then someone would need to look at the TGeo conversion to see why does the layers it constructs have overlapping surfaces).
- Modify the layer construction to slightly shift the surface at creation to avoid overlap (with maybe some warning ?).
- Modify the navigation to have unambiguous surface ordering, but this would probably slow down navigation, which is not ideal. Here is an example of unambiguous ordering that always return the "deepest" surface in the hierarchy :
```
// Sort by object address
std::sort(sIntersections.begin(), sIntersections.end(),
[](const auto& a, const auto& b) { return a.object < b.object; });
// Now look for duplicates. As we just sorted by path length, duplicates
// should be subsequent
auto it = std::unique(
sIntersections.begin(), sIntersections.end(),
[](const SurfaceIntersection& a, const SurfaceIntersection& b) -> bool {
return a.object == b.object;
});

// resize to remove all items that are past the unique range
sIntersections.resize(std::distance(sIntersections.begin(), it));

std::sort(sIntersections.begin(), sIntersections.end(),
[](const auto& a, const auto& b) { return a.object->geometryId() < b.object->geometryId(); });

// sort according to the path length
if (options.navDir == NavigationDirection::Forward) {
std::stable_sort(sIntersections.begin(), sIntersections.end());
} else {
std::stable_sort(sIntersections.begin(), sIntersections.end(), std::greater<>());
}

return sIntersections;
}
```
If anybody has an opinion on this please let me know !

PS : while looking at the navigation I realised we might also have an issue with boundary material being missed when navigating backward through the detector. This is something I need to investigate.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.