-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MTD geometry/reconstruction: define new ETL reco layer geometry, update LGAD pixel structure, activate workflows for scenarios D72 and D73 #31765
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31765/18996
|
A new Pull Request was created by @fabiocos (Fabio Cossutti) for master. It involves the following packages: Configuration/Geometry @perrotta, @civanch, @Dr15Jones, @jordan-martins, @chayanit, @cvuosalo, @wajidalikhan, @ianna, @mdhildreth, @cmsbuild, @makortel, @franzoni, @silviodonato, @kpedro88, @slava77, @jpata, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test workflow 33034.0,33434.0 |
The tests are being triggered in jenkins.
|
-1 Tested at: da87430 CMSSW: CMSSW_11_2_X_2020-10-12-2300 I found follow errors while testing this PR Failed tests: ClangBuild
I found compilation warning while trying to compile with clang. Command used:
See details on the summary page. |
Comparison not run due to Build errors/Fireworks only changes/No short matrix requested (RelVals and Igprof tests were also skipped) |
+1 |
D49 uses C9 while these 2 use C11. The corresponding Era's are Phase2C9 and Phase2C11. I wonder if trigger geometry changed between Phase2C9 and Phase2C11.
…________________________________
From: Kevin Pedro [[email protected]]
Sent: 27 October 2020 15:52
To: cms-sw/cmssw
Cc: Sunanda Banerjee; Mention
Subject: Re: [cms-sw/cmssw] MTD geometry/reconstruction: define new ETL reco layer geometry, update LGAD pixel structure, activate workflows for scenarios D72 and D73 (#31765)
@bsunanda<https://github.com/bsunanda> @jbsauvan<https://github.com/jbsauvan> in the D72 and D73 step2 log files (from test in #31765 (comment)<#31765 (comment)>), I see some missing wafer messages.
D72:
%MSG-w HGCalTriggerGeometry: HGCalVFEProducer:hgcalVFEProducer 26-Oct-2020 22:39:19 CET Run: 1 Event: 3
Found missing wafer (layer=34 u=-8 v=-12) in trigger modules mapping
%MSG
%MSG-w HGCalTriggerGeometry: HGCalVFEProducer:hgcalVFEProducer 26-Oct-2020 22:39:19 CET Run: 1 Event: 3
Found missing wafer (layer=35 u=-12 v=-12) in trigger modules mapping
%MSG
%MSG-w HGCalTriggerGeometry: HGCalVFEProducer:hgcalVFEProducer 26-Oct-2020 22:40:23 CET Run: 1 Event: 9
Found missing wafer (layer=35 u=-8 v=-14) in trigger modules mapping
%MSG
D73:
%MSG-w HGCalTriggerGeometry: HGCalVFEProducer:hgcalVFEProducer 26-Oct-2020 22:39:44 CET Run: 1 Event: 4
Found missing wafer (layer=35 u=-2 v=10) in trigger modules mapping
%MSG
%MSG-w HGCalTriggerGeometry: HGCalVFEProducer:hgcalVFEProducer 26-Oct-2020 22:39:55 CET Run: 1 Event: 5
Found missing wafer (layer=34 u=-8 v=-12) in trigger modules mapping
%MSG
%MSG-w HGCalTriggerGeometry: HGCalVFEProducer:hgcalVFEProducer 26-Oct-2020 22:40:25 CET Run: 1 Event: 8
Found missing wafer (layer=36 u=-4 v=10) in trigger modules mapping
%MSG
%MSG-w HGCalTriggerGeometry: HGCalVFEProducer:hgcalVFEProducer 26-Oct-2020 22:40:35 CET Run: 1 Event: 9
Found missing wafer (layer=35 u=0 v=-12) in trigger modules mapping
%MSG
I'm not sure why these appear in the new workflows and not in e.g. D49, which uses the same HGCal geometry.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#31765 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGMZORPDH5WNK6PZNPORR3SM3NB5ANCNFSM4SOL4G5A>.
|
The trigger mappings used are the same for v11 (C9) and v12 (C11): |
+1 |
+reconstruction
|
@jpata I did a test with wf 33634.0 with PU = 200, using some 10k generated MinBias with the same scenario as pileup file, and running igprof on 100 events. As expected the MTD geometry initialization disappears from the top part of the list.
is the highest MTD contribution to CPU consumption, and concerning this PR:
|
@chaynit questions? Comments? |
+1 |
+1 |
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 be automatically merged. |
PR description:
This PR is built on top of #31710 (here fully embedded), where the new ETL geometries implemented by @icosivi are integrated, and needs #31654 to provide physically meaningful results. Its full scrutiny will benefit from the updated MTD validation for 2-discs ETL prepared by @gsorrentino18 which will be presented in a separate PR.
To complete the basic geometry definition in view of the reconstruction, the logical partitioning of LGADs encoded in the MTD topology (from
mtdParameters.xml
) is updated to the TDR scenario of 2 ROCs per LGAD, 16x32 pixels of 1.3x1.3 mm**2.The heart of the PR is the full update of
RecoMTD/DetLayers
package to add, in parallel to the existing ring-based layer, a sector-based layer suitable to describe both scenario D72 (4 sectors per disc face) and D73 (2 sectors per disc face). A parallel set of classes is built, and the choice of which layer geometry to build is made based on theMTDTopologyMode
parameter provided by the topology ES. In order to describe the sector, which is a bounded disc sector, the existing classes developed for the tracker are used, and moved outside theRecoTracker/TkDetLayers
area into the more standardDataFormats/GeometrySurface
package (where other similar objects are stored).Since at present this code must simultaneously describe two different detector designs, the strategy chosen to search for
compatibleDets
within a sector is kept simple, without and optimized navigation of the module matrix for the time being. As it cannot exploit existing geometrical ordering strategies (as for rings, concentring rings ordered in radius with modules ordered in phi), the numerical sorting of modules within a sectors byETLDetId
provides an approximate ordering of modules (same module number, different types corresponding to left/right side of a modules' strip). This ordering would be pair-wide lexicographic in a x,y matrix of modules, but for the fact that the left and right number of modules does not fully match, as can be seen from a geometry display for one of the D73 discs.When the module of closest approach to the track extrapolation onto the sector surface is identified in a loop probing the distance of all modules, a range of modules around the closest one is explored to search for possible other compatibilities. In order to limit the CPU time taken by the search, a fixed-range window of modules is defined around the closest one, and modules are sorted according to distances are sorted just in that range (and not on the whole sector), so the sorting involves at most 100 elements (in the present implementation), based on the geometry of the setups and observed behaviour.
PR validation:
The standard benchmark workflows for scenarios D72 and D73 , defined already in #31710, are activated in this PR to allow tests to systematically probe also this new code. They have been verified to smoothly run on 100 events. A refreshed dedicated test code is provided in the test area of
RecoMTD/DetLayers
, it has been used to check the structure of layers and behaviour of the search code, and can be easily turned into a unit test like those running already for theGeometry/MTD*
packages. A preliminary check with the updated MTD DQM performed by @gsorrentino18 on a late development stage of this PR showed reasonable results (to be reverified).