acts-project / acts-project/acts

Bug: Probable Issues in Bevelled Cylinder Implementation

Open
#1,238 9 comments 0 reactions 0 assignees View on GitHub
Stale
Dominant language
C++
Stars
131
Forks
276
Avg merge
3d 13h
Merged PRs (30d)
112

Description

Relevant Issue: [1104](https://github.com/acts-project/acts/pull/1104) , [1146](https://github.com/acts-project/acts/issues/1146)

The issue is a summary of the probable problems with PR 1104 which implemented the bevelled cylinder surfaces in ACTS. [Slide 8, 9 here.](https://indico.bnl.gov/event/15458/contributions/62426/attachments/40571/67801/ACTS%20integration%20for%20B0%20tracker.pdf)

In [Acts::CylinderBounds::inside](https://github.com/acts-project/acts/blob/main/Core/src/Surfaces/CylinderBounds.cpp#L40):
1) The vector lposition seems to represent a location in the (R*phi, z) coordinates. This would make sense as a cylindrical surface would unwrap to be a rectangle with the same height as the cylinder but a width = 2pi*R. The calculation of the [localx](https://github.com/acts-project/acts/blob/main/Core/src/Surfaces/CylinderBounds.cpp#L56) variable and its use [localx](https://github.com/acts-project/acts/blob/main/Core/src/Surfaces/CylinderBounds.cpp#L62) does not seem to reflect that. The factor of radial location R is missing.
2) The first check in Acts::CylinderBounds::inside is performed on a [shifted lposition variable which normalizes out the radial factor]( https://github.com/acts-project/acts/blob/main/Core/src/Surfaces/CylinderBounds.cpp#L24). This check does not check for any bevels and seems like it would return true even without checking edge cases for a bevelled cylinder. So, the rest of the code block never gets executed.
3) The block of code following [line 62](https://github.com/acts-project/acts/blob/main/Core/src/Surfaces/CylinderBounds.cpp#L62) and line 65, seems to assume that the boundary of a unwrapped bevelled cylinder is a polygon. This wouldn't be the exact case if one takes into account the evolution of the slope along the boundary surfaces.
![BevelledCylinder](https://user-images.githubusercontent.com/7409132/165825648-92629888-f8d8-4d77-b390-0bf524aee049.JPG)

In [Acts::CylinderBounds::inside3D](https://github.com/acts-project/acts/blob/main/Core/src/Surfaces/CylinderBounds.cpp#L89):
1) The inside3D function has two checks: a fast check that does bevelled cylinders but with full azimuthal range (closed surface) and a slow check that can handle phi segments but doesn't check for bevels. Does the fast check need to be updated to check for finite azimuthal range or is the check here sufficient?
2) In the fast check, should this be [bevelMaxZ](https://github.com/acts-project/acts/blob/main/Core/src/Surfaces/CylinderBounds.cpp#L109)? Seems like a typo.
3) In the fast check, it seems to again only check a projection on the yz-plane and may not be sufficient for a bevelled cylinder with finite azimuthal range.

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.