Bug: Unpredicatable navigation in layer with overlapping surface

[Corentin-Allaire]

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 onpathlenght 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.

[Corentin-Allaire]

We discussed this during the meeting today. @paulgessinger suggest (and I agree) that we should avoid having multiple surface at the same position (we should add a check somewhere) and that in the case where we have only one sensitive surface we should remove the representing (but we are not sure how to do it). @asalzburger do you have any idea ? We were thinking this might not be an issue in you new layerles geometry

[andiwand]

naive thought of mine: could the representative surface be == the actual surface? so we do not have to work around the logic of the representative surface too much and if we have to we can check against the actual surface

[asalzburger]

Ideally we should admit using the representing surface for layers, I suppose - if this is possible, then the problem shouldn't be present anymore.

N.B. the new Experimental::Detector structure doesn't not have representing surfaces anymore.

[Corentin-Allaire]

naive thought of mine: could the representative surface be == the actual surface? so we do not have to work around the logic of the representative surface too much and if we have to we can check against the actual surface

I the specific case I was mentioning here it wouldn't work because the representing is a disk and the sensitive is a rectangle (that is a side effect of our TGeo description not being able to convert rectangle surface into layer)

[stale[bot]]

This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs.