Support rotated HGCal Wafers in Fireworks #38054
Conversation
With the introduction of fully rotated layers in HGCAL, the correct handling and representation in Fireworks became even more complex. The approach followed here is to create fake DetIds for each wafer in the Silicon section of HGCal, have the hexagons correctly computed by the HGCal* geometry-related classes, including the full layer rotation in the CE-H section. The Scintillators geometry is handled using directly tiles, and no changes were required. The routines actually parsing, computing and creating the Fireworks geometry have been changed and update to use those fake wafer-detids.
When querying the geometry for the position of cells in the HGCal detector, the convention used to prepare the Fireworks geometry was that the last point (that is sent in addition to the minimum required to fully describe the global position of each cell) was attached to pass down the information about the thickness of each cell. This PR re-establish that convention that was lost in previous commits. An explicit comment has been added to make that somehow more explicit.
The information for each (full) cell in all Si wafer about its orientation in space is added to the dump of the HGCal Geometry. The information is stored in location `shape[2]` of each valid DetId, and is zeroed for the scintillator tiles. The information is used by some Fireworks plugin to properly render the 3D cells. More information has been added, as comment, directly into the code.
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38054/30138
|
|
A new Pull Request was created by @rovere (Marco Rovere) for master. It involves the following packages:
@civanch, @Dr15Jones, @alja, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-280989/24935/summary.html Comparison SummarySummary:
|
| @@ -402,7 +402,8 @@ HGCalGeometry::CornersVec HGCalGeometry::getNewCorners(const DetId& detid, bool | |||
| co[i] = GlobalPoint( | |||
| (r + signr[i] * dr) * cos(fi + signf[i] * dfi), (r + signr[i] * dr) * sin(fi + signf[i] * dfi), (v.z() + dz)); | |||
| } | |||
| co[ncorner - 1] = co[0]; | |||
| // Used to pass downstream the thickness of this cell | |||
| co[ncorner - 1] = GlobalPoint(0, 0, -2 * dz); | |||
There was a problem hiding this comment.
Would you mind to comment on this line? Do you expect something to change from this line? Thx.
There was a problem hiding this comment.
This is an understanding between geometry and fireworks - in addition to the corners fireworks also needs half the width. Marco has correctly put it in.
|
+1 |
|
+1 |
|
@srimanob Could you please approve this PR. Needed to define V17 geometry |
|
+Upgrade This PR is to update geometry related to the firework on HGCal. |
cmsbuild
added
fully-signed
upgrade-approved
and removed
pending-signatures
upgrade-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This PR addresses the following point:
3DRecHitsrendering of the single cells and tiles is properly done. Comments have been added to the code to alert the developer about the downstream effects in case she wants to change the behaviour.PR validation:
The code has been tested using
D86andD88geometry. No further bugs nor strange behaviour is observed.In particular, layers 28, 30 and 32 show the correct rendering, as shown in the following images:
Moreover, one can see the different orientations of the single cells within the rotated layers with respect to the default orientation, as shown in this picture: