MTD geometry: add BTL topology to MTDTopology, reorganize dependencies #46911

fabiocos · 2024-12-11T10:55:55Z

PR description:

This PR updates the MTDTopology class by adding a BTL part, associating an iphi and ieta index to sensor modules DetId of MTDGeometry, using the same ordering. In order to avoid circular dependencies, MTDTopology is moved from MTDNumberingBuilder, where it was for historical reasons (cloning the tracker geometry structure in old days) to the more appropriate MTDGeometryBuilder. The _cfi.py fragment is kept anyway in the original location (through a python import) to prevent disruption of scores of existing configuration files.

This is the opportunity to:

add a test of BTL navigation in RecoMTD/DetLayers that couples both MTDTopology and MTDTrayBarrelLayer navigation at the same time (in parallel to the existing test for ETL navigation in tracking).
clean some records definition and dependencies;
remove part of codes referring to the obsolete v1 BTL geometry, no more supported and present in CMSSW since this summer;
reorder the MTDGeometry entries running their phi from -pi to pi, as the CMS default used in the RecoMTD/DetLayers construction of BTL, instead of the present order from 0 to 2pi;

In order to keep the unit tests stable at this step, the last point is temporarily undone, and it will be separately proposed together a complete rewriting of the BTL numbering scheme, to be based on top of this PR, where all data references will have anyway to be changed. In this way we reduce the number of space expensive test reference updates.

PR validation:

Code compiles, runs, unit tests are at present providing the expected output, and the yet not used TestBTLNavigation has the expected behaviour:


Test of BTL navigation 



BTL layer   18 at z =           0.00 rods =            108 dets =          10368

Tray    1
 MTDDetTray at  (-115.030,-13.011,0.000)  L/W/T   :         496.07 /           5.47 /           0.38 normal phi =           0.17
    1 BTLDetId 1653824400 iphi/ieta =    1 /    1 side =    0 RU =    2 mod =   22 pos =              (-115.03,-13.01,-245.48) 
...............phishift=  iphi/ieta =  108 /    1  -1 side =    0 RU =    2 mod =   24 pos =              (-115.03,13.01,-245.48) 
...............phishift=  iphi/ieta =    2 /    1   1 side =    0 RU =    2 mod =   23 pos =              (-114.03,-19.03,-245.48) 
...............etashift= -1 out of range
...............etashift=  iphi/ieta =    1 /    2   1 side =    0 RU =    2 mod =   19 pos =              (-115.03,-13.01,-240.32) 
    2 BTLDetId 1653821328 iphi/ieta =    1 /    2 side =    0 RU =    2 mod =   19 pos =              (-115.03,-13.01,-240.32) 
...............phishift=  iphi/ieta =  108 /    2  -1 side =    0 RU =    2 mod =   21 pos =              (-115.03,13.01,-240.32) 
...............phishift=  iphi/ieta =    2 /    2   1 side =    0 RU =    2 mod =   20 pos =              (-114.03,-19.03,-240.32) 
...............etashift=  iphi/ieta =    1 /    1  -1 side =    0 RU =    2 mod =   22 pos =              (-115.03,-13.01,-245.48) 
...............etashift=  iphi/ieta =    1 /    3   1 side =    0 RU =    2 mod =   16 pos =              (-115.03,-13.01,-235.16) 
    3 BTLDetId 1653818256 iphi/ieta =    1 /    3 side =    0 RU =    2 mod =   16 pos =              (-115.03,-13.01,-235.16) 
...............phishift=  iphi/ieta =  108 /    3  -1 side =    0 RU =    2 mod =   18 pos =              (-115.03,13.01,-235.16) 
...............phishift=  iphi/ieta =    2 /    3   1 side =    0 RU =    2 mod =   17 pos =              (-114.03,-19.03,-235.16) 
...............etashift=  iphi/ieta =    1 /    2  -1 side =    0 RU =    2 mod =   19 pos =              (-115.03,-13.01,-240.32) 
...............etashift=  iphi/ieta =    1 /    4   1 side =    0 RU =    2 mod =   13 pos =              (-115.03,-13.01,-230.00) 
    4 BTLDetId 1653815184 iphi/ieta =    1 /    4 side =    0 RU =    2 mod =   13 pos =              (-115.03,-13.01,-230.00) 
...............phishift=  iphi/ieta =  108 /    4  -1 side =    0 RU =    2 mod =   15 pos =              (-115.03,13.01,-230.00) 
...............phishift=  iphi/ieta =    2 /    4   1 side =    0 RU =    2 mod =   14 pos =              (-114.03,-19.03,-230.00) 
...............etashift=  iphi/ieta =    1 /    3  -1 side =    0 RU =    2 mod =   16 pos =              (-115.03,-13.01,-235.16) 
...............etashift=  iphi/ieta =    1 /    5   1 side =    0 RU =    2 mod =   10 pos =              (-115.03,-13.01,-224.84) 
(...)
10366 BTLDetId 1657949072 iphi/ieta =  108 /   94 side =    1 RU =    2 mod =   18 pos =              (-115.03,13.01,235.16) 
...............phishift=  iphi/ieta =  107 /   94  -1 side =    1 RU =    2 mod =   17 pos =              (-114.03,19.03,235.16) 
...............phishift=  iphi/ieta =    1 /   94   1 side =    1 RU =    2 mod =   16 pos =              (-115.03,-13.01,235.16) 
...............etashift=  iphi/ieta =  108 /   93  -1 side =    1 RU =    2 mod =   15 pos =              (-115.03,13.01,230.00) 
...............etashift=  iphi/ieta =  108 /   95   1 side =    1 RU =    2 mod =   21 pos =              (-115.03,13.01,240.32) 
10367 BTLDetId 1657952144 iphi/ieta =  108 /   95 side =    1 RU =    2 mod =   21 pos =              (-115.03,13.01,240.32) 
...............phishift=  iphi/ieta =  107 /   95  -1 side =    1 RU =    2 mod =   20 pos =              (-114.03,19.03,240.32) 
...............phishift=  iphi/ieta =    1 /   95   1 side =    1 RU =    2 mod =   19 pos =              (-115.03,-13.01,240.32) 
...............etashift=  iphi/ieta =  108 /   94  -1 side =    1 RU =    2 mod =   18 pos =              (-115.03,13.01,235.16) 
...............etashift=  iphi/ieta =  108 /   96   1 side =    1 RU =    2 mod =   24 pos =              (-115.03,13.01,245.48) 
10368 BTLDetId 1657955216 iphi/ieta =  108 /   96 side =    1 RU =    2 mod =   24 pos =              (-115.03,13.01,245.48) 
...............phishift=  iphi/ieta =  107 /   96  -1 side =    1 RU =    2 mod =   23 pos =              (-114.03,19.03,245.48) 
...............phishift=  iphi/ieta =    1 /   96   1 side =    1 RU =    2 mod =   22 pos =              (-115.03,-13.01,245.48) 
...............etashift=  iphi/ieta =  108 /   95  -1 side =    1 RU =    2 mod =   21 pos =              (-115.03,13.01,240.32) 
...............etashift=  1 out of range

This reverts commit 387021e. Temporarily silence BTL navigation test for backward compatibility

cmsbuild · 2024-12-11T10:56:21Z

cms-bot internal usage

fabiocos · 2024-12-11T10:58:39Z

type mtd

cmsbuild · 2024-12-11T10:59:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46911/42969

There are other open Pull requests which might conflict with changes you have proposed:
- File RecoTracker/GeometryESProducer/plugins/TrackerMTDRecoGeometryESProducer.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372
- File Validation/MtdValidation/plugins/EtlDigiHitsValidation.cc modified in PR(s): Correction of MTD ETL Validation: hits accumulation on a pixel-basis #43928
- File Validation/MtdValidation/plugins/EtlLocalRecoValidation.cc modified in PR(s): Correction of MTD ETL Validation: hits accumulation on a pixel-basis #43928
- File Validation/MtdValidation/plugins/EtlSimHitsValidation.cc modified in PR(s): Correction of MTD ETL Validation: hits accumulation on a pixel-basis #43928
- File Validation/MtdValidation/plugins/MtdTracksValidation.cc modified in PR(s): Correction of MTD ETL Validation: hits accumulation on a pixel-basis #43928

cmsbuild · 2024-12-11T10:59:26Z

A new Pull Request was created by @fabiocos for master.

It involves the following packages:

Geometry/MTDGeometryBuilder (upgrade, geometry)
Geometry/MTDNumberingBuilder (upgrade, geometry)
Geometry/Records (geometry)
RecoLocalFastTime/FTLClusterizer (upgrade, reconstruction)
RecoLocalFastTime/FTLCommonAlgos (upgrade, reconstruction)
RecoLocalFastTime/FTLRecProducers (upgrade, reconstruction)
RecoMTD/DetLayers (upgrade, reconstruction)
RecoTracker/GeometryESProducer (reconstruction)
RecoTracker/TkDetLayers (reconstruction)
SimFastTiming/FastTimingCommon (upgrade, simulation)
SimFastTiming/MtdAssociatorProducers (upgrade, simulation)
SimGeneral/CaloAnalysis (simulation)
Validation/MtdValidation (dqm)

@Dr15Jones, @Moanwar, @antoniovagnerini, @bsunanda, @civanch, @cmsbuild, @jfernan2, @kpedro88, @makortel, @mandrenguyen, @mdhildreth, @rseidita, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @ReyerBand, @VinInn, @VourMa, @argiro, @bsunanda, @dgulhan, @felicepantaleo, @gpetruc, @martinamalberti, @missirol, @mmusich, @mtosi, @rchatter, @rovere, @slomeo, @thomreis, @wang0jin this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

fabiocos · 2024-12-11T11:02:15Z

please test

cmsbuild · 2024-12-11T13:22:59Z

+1

Size: This PR adds an extra 228KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3504b6/43377/summary.html
COMMIT: 4193a38
CMSSW: CMSSW_15_0_X_2024-12-10-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46911/43377/install.sh to create a dev area with all the needed externals and cmssw changes.

DAS Queries: The DAS query tests failed, see the summary page for details.

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
Reco comparison results: 286 differences found in the comparisons
DQMHistoTests: Total files compared: 47
DQMHistoTests: Total histograms compared: 3623082
DQMHistoTests: Total failures: 2504
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3620558
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 46 files compared)
Checked 206 log files, 177 edm output root files, 47 DQM output files
TriggerResults: found differences in 1 / 45 workflows

jfernan2 · 2024-12-11T17:40:50Z

+1

civanch · 2024-12-12T05:35:43Z

+1

Moanwar · 2024-12-12T12:01:48Z

+Upgrade

antoniovagnerini · 2024-12-12T15:08:18Z

+dqm

cmsbuild · 2024-12-12T15:08:44Z

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. @mandrenguyen, @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

mandrenguyen · 2024-12-12T15:54:11Z

+1

smuzaffar · 2024-12-13T07:56:47Z

@fabiocos , this PR is breaking debug IBs and also clang static checks ( which were failed during PR tests too )[a]. Can you please provide a fix as soon as possible

[a] https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3504b6/43377/llvm-analysis/runStaticChecks.log

>> Analyzing  src/Geometry/Records/src/PEcalPreshowerRcd.cc 
src/Geometry/MTDNumberingBuilder/plugins/DDCmsMTDConstruction.cc:211:90: error: use of undeclared identifier 'makempiToppi'
  211 |           << " " << it->type() << " " << it->translation().z() << " " << convertRadToDeg(makempiToppi(it->phi()))
      |                                                                                          ^
src/Geometry/MTDNumberingBuilder/plugins/DDCmsMTDConstruction.cc:365:90: error: use of undeclared identifier 'makempiToppi'
  365 |           << " " << it->type() << " " << it->translation().z() << " " << convertRadToDeg(makempiToppi(it->phi()))
      |                                                                                          ^
2 errors generated.

fabiocos added 8 commits December 3, 2024 14:18

Move MTDTopology to MTDGeometryBuilder, add methods for BTL topology

c93034d

Add MTDDigiGeometryRecord to MTDTopology, adapt all the rest

2e1c7f6

Implement BTL topology navigation

24b649f

Add test for BTL topology navigation in parallel to tracking geometry

c32dd11

Move BTL phi order from 0 to 2pi to -pi to +pi

387021e

Fixes and debugging printouts

a372e6b

Revert "Move BTL phi order from 0 to 2pi to -pi to +pi"

06d30e1

This reverts commit 387021e. Temporarily silence BTL navigation test for backward compatibility

Remove obsolete v1 BTL geometry from constructions

4193a38

cmsbuild added this to the CMSSW_15_0_X milestone Dec 11, 2024

cmsbuild added reconstruction-pending simulation-pending dqm-pending geometry-pending pending-signatures tests-pending orp-pending upgrade-pending code-checks-pending tracking labels Dec 11, 2024

cmsbuild added the mtd label Dec 11, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 11, 2024

cmsbuild added tests-started and removed tests-pending labels Dec 11, 2024

cmsbuild added tests-approved and removed tests-started labels Dec 11, 2024

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Dec 11, 2024

cmsbuild added geometry-approved simulation-approved and removed simulation-pending geometry-pending labels Dec 12, 2024

cmsbuild added upgrade-approved and removed upgrade-pending labels Dec 12, 2024

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Dec 12, 2024

cmsbuild added orp-approved and removed orp-pending labels Dec 12, 2024

cmsbuild merged commit ab76956 into cms-sw:master Dec 12, 2024
11 checks passed

cmsbuild mentioned this pull request Dec 12, 2024

Add -fno-omit-frame-pointer as default flag for compilation cms-sw/cmsdist#9565

Closed

fabiocos deleted the fc-mtdtopo20241202 branch December 13, 2024 08:55

fabiocos mentioned this pull request Dec 16, 2024

MTD geometry: activate the reordering of MTD reconstruction geometry #46963

Closed

cquarant mentioned this pull request Dec 17, 2024

MTD Geometry - Numbering scheme update #46977

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MTD geometry: add BTL topology to MTDTopology, reorganize dependencies #46911

MTD geometry: add BTL topology to MTDTopology, reorganize dependencies #46911

fabiocos commented Dec 11, 2024 •

edited

Loading

cmsbuild commented Dec 11, 2024 •

edited

Loading

fabiocos commented Dec 11, 2024

cmsbuild commented Dec 11, 2024

cmsbuild commented Dec 11, 2024

fabiocos commented Dec 11, 2024

cmsbuild commented Dec 11, 2024

jfernan2 commented Dec 11, 2024

civanch commented Dec 12, 2024

Moanwar commented Dec 12, 2024

antoniovagnerini commented Dec 12, 2024

cmsbuild commented Dec 12, 2024

mandrenguyen commented Dec 12, 2024

smuzaffar commented Dec 13, 2024 •

edited

Loading

MTD geometry: add BTL topology to MTDTopology, reorganize dependencies #46911

MTD geometry: add BTL topology to MTDTopology, reorganize dependencies #46911

Conversation

fabiocos commented Dec 11, 2024 • edited Loading

PR description:

PR validation:

cmsbuild commented Dec 11, 2024 • edited Loading

fabiocos commented Dec 11, 2024

cmsbuild commented Dec 11, 2024

cmsbuild commented Dec 11, 2024

fabiocos commented Dec 11, 2024

cmsbuild commented Dec 11, 2024

Comparison Summary

jfernan2 commented Dec 11, 2024

civanch commented Dec 12, 2024

Moanwar commented Dec 12, 2024

antoniovagnerini commented Dec 12, 2024

cmsbuild commented Dec 12, 2024

mandrenguyen commented Dec 12, 2024

smuzaffar commented Dec 13, 2024 • edited Loading

fabiocos commented Dec 11, 2024 •

edited

Loading

cmsbuild commented Dec 11, 2024 •

edited

Loading

smuzaffar commented Dec 13, 2024 •

edited

Loading