Fix PixelCPEBase::fillDetParams() for Phase-2 IT, using new GeomDetEnumerators methods

[mmusich]

PR description:

While trying to move the reading of the Lorentz Angle for the phase-2 Inner Tracker detector to DataBase, we noticed that when not providing the LA value for all the DetIds (even OuterTracker ones), the SiPixelRecHitConverter was complaining with errors of the type:

%MSG-e SiPixelLorentzAngle: SiPixelRecHitConverter:siPixelRecHits 04-Jul-2019 18:56:11 CEST Run: 1 Event: 3
SiPixelLorentzAngle for DetID 419697669 is not stored
%MSG

while not effectively crashing or giving bogus results.
We traced back the issue to the logic implemented in the fillDetParams method of PixelCPEBase which looks for the first not-Inner Tracker DetUnit to determine the size of the DetUnit list to loop over.
Currently the list is stopped at the first not Strip DetUnit via the method GeomDetEnumerators::isTrackerStrip.
This unfortunately doesn't work in the case of the Phase-2 detector as the GeomDetEnumerators::isTrackerStrip takes into account only the current Strip Subdetectors.
We solved this inconvenience by add new methods:

• isInnerTracker
• isOuterTracker

in GeomDetEnumerators and GeomDetType and use them to fix the logic to hold also for Phase-2.
These methods seem to be quite handy, therefore I am wondering if there is a specific reason why they were not implemented before?
The reason why the current setup is not producing these error messages is because the Lorentz Angle used in the reconstruction comes from a Fake conditions ESProducer: SiPixelFakeLorentzAngleESSource which produces a value for all the DetId list from “Skimmed Geometry” external file (so all of them).
Some more information about this can be found in this presentation at the Tracker Phase-2 simulation,modeling and performance meeting .

I profit of this PR to make two other light fixes:

• apply the same logic fix also the PixelCPEClusterRepair::fill2DTemplIDs() method (72cee37)
• purely technical change, use MagneticField::nominalValue() as suggested here (59ee713)

PR validation:

Code proposes passes all tests of:

runTheMatrix.py -l limited -t 4 -j 8

if this PR is a backport please specify the original PR:

This is not a backport.

cc:
@tsusa, @pmaksim1, @OzAmram, @cmantill, @emiglior

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27448/10760

• This PR adds an extra 28KB to repository

[cmsbuild]

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

Geometry/CommonDetUnit
RecoLocalTracker/SiPixelRecHits

@perrotta, @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @dkotlins, @gpetruc, @ebrondol, @threus this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[VinInn]

These methods seem to be quite handy, therefore I am wondering if there is a specific reason why they were not implemented before?

Laziness...

[perrotta]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1346/console Started: 2019/07/06 09:38

[cmsbuild]

+1
Tested at: 59ee713
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cba38e/1346/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cba38e/1346/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 6280 differences found in the comparisons
• DQMHistoTests: Total files compared: 33
• DQMHistoTests: Total histograms compared: 3160399
• DQMHistoTests: Total failures: 21580
• DQMHistoTests: Total nulls: 3
• DQMHistoTests: Total successes: 3138482
• DQMHistoTests: Total skipped: 334
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -0.012 KiB( 32 files compared)
• DQMHistoSizes: changed ( 136.85 ): -0.012 KiB JetMET/SUSYDQM
• Checked 137 log files, 14 edm output root files, 33 DQM output files

[perrotta]

@mmusich
Test outputs show a lot of differences in the non-Phase2 workflows (and none in the Phase2 ones...).
According to the PR description this is not what we should have expected: please have a look.

[mmusich]

Test outputs show a lot of differences in the non-Phase2 workflows (and none in the Phase2 ones...).
According to the PR description this is not what we should have expected: please have a look.

@perrotta: please read carefully again the description. No changes are expected in the Phase2 workflows because of how those workflows are configured.
As for the changes in the Run2 workflows, since there is no change as far as I can see in the Run1 and in the Phase2 I presume it's due to the changes in the PixelCPEClusterRepair which is run only in Run2. I'll have a look.

[perrotta]

please test workflow 138.1

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1352/console Started: 2019/07/07 16:40

[cmsbuild]

+1
Tested at: 92c174d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cba38e/1352/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cba38e/1352/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 33
• DQMHistoTests: Total histograms compared: 3160399
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3160064
• DQMHistoTests: Total skipped: 334
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
• Checked 137 log files, 14 edm output root files, 33 DQM output files

[perrotta]

+1

• Replaced the usage of the GeomDetEnumerators::isTrackerStrip method (which would give buggy results in Phase2) with newly defined isOuterTrackerone
• isInnerTracker implemented as well, for possible future usage
• use MagneticField::nominalValue() to check whether the B is on, instead of the more complex MagneticField::inTesla(...) method
• Jenkins tests pass and show no differences, as they should

[cvuosalo]

+1

[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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

[fabiocos]

+1