Implement curved RSU structure for the three layers of vertex barrel trackers #803

ShujieL · 2024-11-11T01:58:44Z

Briefly, what does this PR introduce?

created a new vertex barrel geometry file vertex_barrel_curved.xml with a new geo plugin to support smooth cylindrical surface and build-in inactive areas.
This work is a follow-up of #759 which creates flat sensors and barrel with staves.

What kind of change does this PR introduce?

Bug fix (issue #__)
[ x ] New feature (issue #__)
Documentation update
Other: __

Please check if this PR fulfills the following:

Tests for the changes have been added
Documentation has been added / updated
[ x] Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

The # of hits on vertex barrels will reduce by a few percent due to the inactive areas.
Didn't see a significant change in tracking efficiency. Performance study on going.

…ethod has unknown offset between tube components. Will switch to part-by-part frame construction instead

…e segmentation for tubes

…with ACTS e.g. only half of the unit is valid when making sourcelinks

… inactive areas

for more information, see https://pre-commit.ci

ShujieL · 2024-11-11T06:35:44Z

VertexBarrelEnvelope_length and VertexTrackingRegion_length are not used anywhere. Can we remove them from dump-parameter-table?

Also, not sure why we keep VertexCheck in the xml file.

ShujieL · 2024-11-11T06:38:28Z

Also, validate-material-map failed due to "Propagation reached the step count limit of 1000 (did 1000 steps)". I remember seeing this error many times in the past but they didn't interrupt the program, a material map would still be generated. Any idea on how to deal with this? @wdconinc

wdconinc · 2024-11-12T00:32:18Z

Also, validate-material-map failed due to "Propagation reached the step count limit of 1000 (did 1000 steps)". I remember seeing this error many times in the past but they didn't interrupt the program, a material map would still be generated. Any idea on how to deal with this? @wdconinc

no time to look into it until next week (not even sure then)

veprbl · 2024-11-12T04:16:51Z

Also:

MultiSegmentation ERROR Invalid sub-segmentation identifier!
VertexBarrelHandler ERROR cellID: failed to access segmentation for VolumeID: 000000000103B21F [17019423]  [MultiSegmentation: Invalid sub-segmentation identifier!]
VertexBarrelHandler ERROR ....... G4-local:   (49.820797, 1.685985, 1.892819) G4-global:   (34.036452, 36.420795, -19.331181)
VertexBarrelHandler ERROR ....... TGeo-local: (4.982080, 0.168598, 0.189282) TGeo-global: (3.403645, 3.642079, -1.933118)
VertexBarrelHandler ERROR ....... Pre-step:  AV_1555!Module2_upper_58#58!RSU_upper_3#3  SD: VertexBarrel
VertexBarrelHandler ERROR ....... Post-step: AV_11865!VertexBarrelSubAssembly_3#3!VertexBarrel_layer2_1#1  SD: 
DDSim            ERROR Simulation failed!
Geant4Kernel     FATAL +++ Exception while simulating:MultiSegmentation: Invalid sub-segmentation identifier!

for more information, see https://pre-commit.ci

wdconinc · 2024-11-15T22:13:05Z

configurations/craterlake.yml

-    vertex_barrel:
+    vertex_barrel_curved:


The default name should be the best implementation. We should want people to be able to rely on the fact that vertex_barrel is just going to be the best possible implementation of the vertex barrel. Any suffixes should be exceptions, not what we want people to use by default. I would suggest that you rename the previous vertex_barrel to something like vertex_barrel_flat_staves or something.

wdconinc · 2024-11-15T22:23:26Z

compact/tracking/vertex_barrel_curved.xml

+          grid_size_phi="0.02*mm/VertexBarrelSeg3_r" grid_size_z="0.02*mm"
+          radius="VertexBarrelSeg3_r" />
+      </segmentation>
+      <id>system:8,layer:4,module:12,sensor:3,phi:30:-18,z:-16</id>


Does phi,z not fit in 32 bits? Based on my quick calculation, phi is still within 16 bits for L2.

Also, you're allocating 4 bits for layers, which allows for 16 layers. You only use 3 layers, so 3 bits would be sufficient.

Also, your bit fields don't add up cleanly to 64 due to sensor being 3 bits wide.

wdconinc · 2024-11-15T22:24:38Z

compact/tracking/vertex_barrel_curved.xml

+    <constant name="VertexBarrelSeg3_r" value="VertexBarrelMod3_rmin+SiVertexSensor_thickness/2.0" />
+
+    <documentation>
+      - Currently there are 3 sensor layers: Layer 1,2,3 = L0, L1, L2.


It will be confusing to have VertexBarrelSeg2_r refer to L1 under this off-by-one numbering scheme. Please use VertexBarrelSegL1_r etc to avoid this confusion.

wdconinc · 2024-11-15T22:25:22Z

compact/tracking/vertex_barrel_curved.xml

+    <constant name="VertexBarrelLayer3_rmax"  value="VertexBarrelLayer3_rmin + VertexBarrelLayer_thickness" />
+
+    <comment> ensure we are within the vertex envelope with some margin. </comment>
+    <constant name="VertexCheck" value="sqrt(VertexBarrel_rmax - VertexBarrelLayer3_rmax - .1*cm)" />


This is unused.

wdconinc · 2024-11-15T22:28:16Z

compact/tracking/vertex_barrel_curved.xml

+  <readouts>
+    <readout name="VertexBarrelHits">
+      <segmentation type="MultiSegmentation" key="layer">
+        <segmentation name="L0" type="CylindricalGridPhiZ" key_value="1"


Again, a mismatch between L0 and key_value 1. Just use key_value 0 for L0, etc. This will avoid mistakes.

src/SVTBarrelTracker_geo.cpp

wdconinc · 2024-11-15T22:33:13Z

src/SVTBarrelTracker_geo.cpp

Avoid naming the plugin to the specific detector. As far as I understand this will need to be done for other detectors too. We should try to make sure we can reuse this geometry plugin for the other detectors too. Therefore, naming this as CurvedBarrelTracker_geo or so is probably useful to consider.

wdconinc · 2024-11-15T22:34:17Z

src/SVTBarrelTracker_geo.cpp

+ * - Designed to process "vertex_barrel_curved.xml"
+ *
+ * - Derived from "BarrelTrackerWithFrame_geo.cpp".
+ * - Build-in RSU structure with 12 tiles and inactive areas


Number of tiles is likely configurable parameter, not inherent to the geometry plugin.

wdconinc · 2024-11-15T22:35:36Z

src/SVTBarrelTracker_geo.cpp

+  Assembly assembly(det_name);
+
+  sens.setType("tracker");
+


I noticed you removed the support and frame sections here. Did you not want to keep the support for this plugin?

wdconinc · 2024-11-15T22:38:27Z

src/SVTBarrelTracker_geo.cpp

+    double m_rmin    = x_mod.rmin();
+    double m_length  = x_mod.length();
+    double m_width   = x_mod.width();
+    module_length[m_nam].push_back(m_length);


I don't understand why the module length is necessary as a map to vector. Will there be multiple modules with the same name? Do you need the length outside of this scope?

Co-authored-by: Wouter Deconinck <[email protected]>

ShujieL added 4 commits October 28, 2024 10:56

initial implementation of curved SVT with RSU. the SubtractionSolid m…

7b115d1

…ethod has unknown offset between tube components. Will switch to part-by-part frame construction instead

build the curved sensor by assembling all parts. still need to fix th…

839e9f1

…e segmentation for tubes

the four tile one module design with updated L2 radius. Having issue …

b30007d

…with ACTS e.g. only half of the unit is valid when making sourcelinks

update three SVT layers with curved RSU structure with four tiles and…

70fe21d

… inactive areas

ShujieL requested a review from wdconinc November 11, 2024 01:58

[pre-commit.ci] auto fixes from pre-commit.com hooks

f127bbd

for more information, see https://pre-commit.ci

ShujieL self-assigned this Nov 11, 2024

github-actions bot added topic: tracking topic: forward Positive-rapidity detectors (hadron-going side) labels Nov 11, 2024

ShujieL removed the topic: forward Positive-rapidity detectors (hadron-going side) label Nov 11, 2024

github-actions bot added the topic: forward Positive-rapidity detectors (hadron-going side) label Nov 11, 2024

ShujieL and others added 3 commits November 14, 2024 16:49

fixed a bug in segmentation syntax. Also updated comments

6e3ca57

[pre-commit.ci] auto fixes from pre-commit.com hooks

bfb8b56

for more information, see https://pre-commit.ci

Merge branch 'main' into svt_curved

5e60a76

wdconinc reviewed Nov 15, 2024

View reviewed changes

Update src/SVTBarrelTracker_geo.cpp

2f9873e

Co-authored-by: Wouter Deconinck <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement curved RSU structure for the three layers of vertex barrel trackers #803

Implement curved RSU structure for the three layers of vertex barrel trackers #803

ShujieL commented Nov 11, 2024 •

edited

Loading

ShujieL commented Nov 11, 2024

ShujieL commented Nov 11, 2024

wdconinc commented Nov 12, 2024

veprbl commented Nov 12, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

wdconinc Nov 15, 2024

Implement curved RSU structure for the three layers of vertex barrel trackers #803

Are you sure you want to change the base?

Implement curved RSU structure for the three layers of vertex barrel trackers #803

Conversation

ShujieL commented Nov 11, 2024 • edited Loading

Briefly, what does this PR introduce?

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

ShujieL commented Nov 11, 2024

ShujieL commented Nov 11, 2024

wdconinc commented Nov 12, 2024

veprbl commented Nov 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShujieL commented Nov 11, 2024 •

edited

Loading