Feat: Radial bounds in DD4hep plugin

[whit2333]

See this pull request for some background.

For some reason the positive endcap rmin is not set correctly. It should be 22 below but it is not set as the value but the envelope. I think it is related to the rather confusing use of envelope and extent in Protolayer.

If I insert here the following line:

pl.extent.ranges[Acts::binR] = {rMin, rMax};

Then things seem to work.

...
01:06:45    D2A_LAC        VERBOSE   Creating a disk Layer:
01:06:45    D2A_LAC        VERBOSE    - at Z position    = 176.5
01:06:45    D2A_LAC        VERBOSE    - from Z min/max   = 176 / 177
01:06:45    D2A_LAC        VERBOSE    - with Z thickness = 3
01:06:45    D2A_LAC        VERBOSE      - incl envelope  = 1 / 1
01:06:45    D2A_LAC        VERBOSE    - with R min/max   = 0 (-22) / 150 (+0)
01:06:45    D2A_LAC        VERBOSE    - with phi min/max = -3.14159 / 3.14159
01:06:45    D2A_LAC        VERBOSE    - # of modules     = 1
01:06:45    D2A_SAC        VERBOSE   Creating a SurfaceArray on a disc
01:06:45    D2A_SAC        VERBOSE   Create equidistant binning Axis for binned SurfaceArray
01:06:45    D2A_SAC        VERBOSE      BinningValue: 3
01:06:45    D2A_SAC        VERBOSE      (binX = 0, binY = 1, binZ = 2, binR = 3, binPhi = 4, binRPhi = 5, binH = 6, binEta = 7)
01:06:45    D2A_SAC        VERBOSE      Number of bins: 1
01:06:45    D2A_SAC        VERBOSE      (Min/Max) = (0/150)
01:06:45    D2A_SAC        VERBOSE   Create equidistant binning Axis for binned SurfaceArray
01:06:45    D2A_SAC        VERBOSE      BinningValue: 4
01:06:45    D2A_SAC        VERBOSE      (binX = 0, binY = 1, binZ = 2, binR = 3, binPhi = 4, binRPhi = 5, binH = 6, binEta = 7)
01:06:45    D2A_SAC        VERBOSE      Number of bins: 1
01:06:45    D2A_SAC        VERBOSE      (Min/Max) = (-3.14159/3.14159)
01:06:45    D2A_SAC        VERBOSE   - z-position of disk estimated as 176.5
01:06:45    D2A_SAC        VERBOSE    -- with 1 surfaces.
01:06:45    D2A_SAC        VERBOSE    -- with r x phi  = 1 x 1 = 1 bins.
01:06:45    D2A_SAC        VERBOSE   Complete binning by filling closest neighbour surfaces into empty bins.
01:06:45    D2A_SAC        VERBOSE          filled  : 0 (includes under/overflow)
01:06:45    D2A_LAC        VERBOSE   Performing consistency check
01:06:45    D2A_LAC        VERBOSE    - Checked 1 valid bins
01:06:45    D2A_LAC        VERBOSE    -- All bins point to a surface
01:06:45    D2A_LAC        VERBOSE    -- All sensitive surfaces are accessible through binning.

****************************************************
*  A runtime error has occured :
*    RadialBounds: invalid radial setup
*  the program will have to be terminated - sorry.
****************************************************

[paulgessinger]

Are the extents auto-determined here and that's what's failing here?

IIRC ProtoLayer::measure is responsible for trying to automatically determine the extent from the contained surfaces and the envelopes are (should be) a configuration parameter.

[paulgessinger]

Ok, the spot where you insert the extent addition shouldn't have to set up the extent anymore, I think. This line calls the ProtoLayer constructor with a vector of sensitive surfaces which calls ProtoLayer::measure.

The log output mentions there only being one sensitive surface. Can you help me understand the structure here? Maybe ProtoLayer::measure fails to correctly determine the size of the contained surface.

[whit2333]

@paulgessinger it is this line in the log that is the problem:

01:06:45    D2A_LAC        VERBOSE    - with R min/max   = 0 (-22) / 150 (+0)

I believe the 0 (-22) should be 22 (-0).

The layer has only a single sensitive surface (another disk).

If you look closely at this output you will see the two endcaps:

23:20:42    D2A_L:AVerte   VERBOSE    Received layers for negative volume -> creating disc layers
23:20:42    D2A_LAC        VERBOSE   Creating a disk Layer:
23:20:42    D2A_LAC        VERBOSE    - at Z position    = -176.5
23:20:42    D2A_LAC        VERBOSE    - from Z min/max   = -177 / -176
23:20:42    D2A_LAC        VERBOSE    - with Z thickness = 1
23:20:42    D2A_LAC        VERBOSE      - incl envelope  = 0 / 0
23:20:42    D2A_LAC        VERBOSE    - with R min/max   = 25 (-0) / 150 (+0)
23:20:42    D2A_LAC        VERBOSE    - with phi min/max = 3.14159 / 3.14159
23:20:42    D2A_LAC        VERBOSE    - # of modules     = 1

This makes sense. Then...

23:20:42    D2A_L:AVerte   VERBOSE    Received layers for positive volume -> creating disc layers
23:20:42    D2A_LAC        VERBOSE   Creating a disk Layer:
23:20:42    D2A_LAC        VERBOSE    - at Z position    = 176.5
23:20:42    D2A_LAC        VERBOSE    - from Z min/max   = 176 / 177
23:20:42    D2A_LAC        VERBOSE    - with Z thickness = 1
23:20:42    D2A_LAC        VERBOSE      - incl envelope  = 0 / 0
23:20:42    D2A_LAC        VERBOSE    - with R min/max   = 0 (-0) / 150 (+0)
23:20:42    D2A_LAC        VERBOSE    - with phi min/max = -3.14159 / 3.14159

The two endcaps should be the same (mirrored) but the positive endcap has a problem. It is setting R min to 0 when it should be 25.

[asalzburger]

I see, there's probably a bug in there, as the extend evaluation tries to guarantee that the extremes at pi, 0, 2pi are evaluated, probably that's what the flipping kills ...

[whit2333]

@asalzburger What is the difference between the extent and the envelopes of the ProtoLayer?

[asalzburger]

The envelope is an additional clearing around the outermost values, e.g 1mm. The extend should estimate the actual space that is occupied by this shape - the clearing (envelope) is then added to it.

[whit2333]

@asalzburger thanks. I think as @paulgessinger suggested, there might be a problem with ProtoLayer::measure which leads me to Acts::Extent Acts::Polyhedron::extent(const Transform3& transform) const.

Are there any assumptions that go into evaluating this line?

edit: That line seems to set rmin=0 (as well as set the phi limits to +-pi which previously were +pi to +pi). Nothing can be less than rmin=0 right?

[asalzburger]

Mmm, this line tries to estimate if the generated mesh (polygon) covers the origin of the surface and then forces the min radius to be 0, but if you have a disc (ring) structure it should actually find out that the origin is never covered, so the mesh generation might have a bug due to the pi flip.

Once it's set to 0, I agree the envelope should (must) be ignored.

Btw, the same code (generating the mesh) is used for the obj display, though there the number of segments for approximating a disc as a triangular mesh is higher, and not just the extremes.

I suppose we can write easily a few lines of code to nail down the issue.

[asalzburger]

So, summarising, we have two independent problems:

• the envelope should not be applied on r if that's already 0
• the r_min estimation of the flipped disk is wrong (likely due to phi flip)

[whit2333]

@asalzburger I think this is correct. Do you have a good idea how to fix these issues or should I try to pin it down?

[asalzburger]

Divide and conquer:

• I think you already started to work on a PR that omits the envelope in case r_min = 0, I think that's well justified, so you can just go ahead and submit this PR

• I will tackle the r_min estimation with another PR, as I said, it should be possible to even display what's going on. Can you - just for me being able to make a mock-up, either:

  • ->Print() of the TGeo object in your geometry that is causing the problem
  • give me the shape ('TGeoTubs'?), and transform ( is it simply flipped with z axis pointing into negative z ?) ... r_min, r_max should be irrelevant

[paulgessinger]

@asalzburger @whit2333 is this still a problem after #829 and #820?

[whit2333]

I think so. When I realized that my "simplified disk geometry" was the source of the problems I switched the geometry construction. However, I am still having endcap geometry problems (see #860). Not sure if any of this is related.

[stale[bot]]

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

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

[paulgessinger]

@wdconinc volunteered to have a look at this. Let us know if this is still relevant.

[wdconinc]

This is not something that is affecting us anymore in EIC.

We did move away from simple disks for the endcaps, so I cannot definitely state that the issue is gone. However, from my reading above and correlating with the PRs and our change, it seems all parts are resolved by #829 and #820.

The issue is likely not relevant anymore and can be closed.

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

[AJPfleger]

@wdconinc Thanks for confirming again in today's meeting, that it can be closed.