MTD geometry and reconstruction: store ETL layout logic into MTDTopology, use for ETL layers navigation in reconstruction

[fabiocos]

PR description:

This PR updates the ETL DetSector navigation algorithm originally deployed in #31765 with the aim of recovering those GeomDets that are possibly there in particular cases (very asymmetric trac extrapolation uncertainties in x and y), trying at the same time to make the navigation heavier.

A description of the strategy is provided by @martatornago in https://indico.cern.ch/event/1033599/contributions/4340606/attachments/2236330/3791605/ETLNavigationAlgorithm.pdf .

The logical layout of sensors in a MTDDetSector (a x-y matrix of alternated left-right type rows of modules) is loaded from the xml geometry and stored in the refreshed MTDTopology class of Geometry/MTDNumberingBuilder, and functions are provided to search for close modules both in the horizontal and vertical direction.

This functionality is used inside the method compatibleDets of MTDDetSector in RecoMTD/DetLayers replacing the previous algorithm.

The test code in this package is expanded, and used to build a unit test, whose reference needs to be added into cms-data/Geometry-TestReference.

According to the Timing service, the time taken by trackExtenderWithMTD with this new version of the navigation is within 3% of the previous one. A more detailed analysis will be possible when igprof is fixed.

PR validation:

Code compiles, the unit test provides the expected results, extensively checked especially in sector borders, and the wf 34634.0 shows the expected behaviour: all previously found modules are collected by the new algorithm, and more are found in the case previously missed of very asymmetric extrapolation uncertainties. The DQM output on 100 TTbar events does not show any significant difference, beyond what described.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33598/22412

• This PR adds an extra 92KB to repository

[cmsbuild]

A new Pull Request was created by @fabiocos (Fabio Cossutti) for master.

It involves the following packages:

Geometry/MTDCommonData
Geometry/MTDGeometryBuilder
Geometry/MTDNumberingBuilder
RecoMTD/DetLayers

@perrotta, @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @kpedro88, @cmsbuild, @srimanob, @mdhildreth, @slava77, @jpata can you please review it and eventually sign? Thanks.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[fabiocos]

please test with cms-data/Geometry-TestReference#8

@parbol FYI

[cmsbuild]

-1

Failed Tests: UnitTests RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d8d798/14775/summary.html
COMMIT: 7ef7284
CMSSW: CMSSW_12_0_X_2021-05-01-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33598/14775/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testPhase2PixelNtuple had ERRORS

RelVals

• 23234.0

----- Begin Fatal Exception 01-May-2021 21:14:38 CEST-----------------------
An exception of category 'MTDTopologyEP' occurred while
   [0] Running EventSetup component MTDTopologyEP/'mtdTopology
Exception Message:
Inconsistent size of ETL structure arrays
----- End Fatal Exception -------------------------------------------------

• 28234.0

----- Begin Fatal Exception 01-May-2021 21:17:53 CEST-----------------------
An exception of category 'MTDTopologyEP' occurred while
   [0] Running EventSetup component MTDTopologyEP/'mtdTopology
Exception Message:
Inconsistent size of ETL structure arrays
----- End Fatal Exception -------------------------------------------------

RelVals-INPUT

• 23234.023234.0_TTbar_14TeV+2026D49+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step2_TTbar_14TeV+2026D49+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log
• 23234.10323234.103_TTbar_14TeV+2026D49Aging3000+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step2_TTbar_14TeV+2026D49Aging3000+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log
• 23234.2123234.21_TTbar_14TeV+2026D49_ProdLike+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+MiniAOD/step2_TTbar_14TeV+2026D49_ProdLike+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+MiniAOD.log

Expand to see more relval errors ...

• 23234.9
• 23434.21
• 23434.9921
• 23434.99
• 23434.999

[fabiocos]

hold

all the errors are related to the legacy D49 scenario, where the new MTDTopology does not hold. I will add a protection for this case, hoping that we may finally get rid soon of it.

[cmsbuild]

Pull request has been put on hold by @fabiocos
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33598/22423

• This PR adds an extra 12KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33598/22423/code-format.patch
  e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33598/22423/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d8d798/15091/summary.html
COMMIT: 2459b75
CMSSW: CMSSW_12_0_X_2021-05-13-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33598/15091/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 85 differences found in the comparisons
• DQMHistoTests: Total files compared: 37
• DQMHistoTests: Total histograms compared: 2648242
• DQMHistoTests: Total failures: 577
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 2647642
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.004 KiB( 36 files compared)
• DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
• Checked 155 log files, 37 edm output root files, 37 DQM output files
• TriggerResults: no differences found

[srimanob]

+Upgrade

[fabiocos]

@cvuosalo @jpata since your last signature only a comment line was removed

[cvuosalo]

+1

[perrotta]

+reconstruction
(Only removed two already commented out lines since previous reco signature - from @jpata - in #33598 (comment))

[cmsbuild]

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[qliphy]

+1

[mrodozov]

the RecoMTDDetLayersTestDriver test started failing for both PPC and Arm after it was added in this PR

[fabiocos]

@mrodozov thanks for reporting, it looks like a rounding issue in the calculation of R at https://github.com/cms-sw/cmssw/blob/master/RecoMTD/DetLayers/test/MTDRecoGeometryAnalyzer.cc#L135

[fabiocos]

addressed in #33762