Addition of the MTD to the tracker Navigation #33272

parbol · 2021-03-25T16:16:39Z

This PR implements the addition of the MTD DetLayers into the GeometricSearchTracker to be used in the tracker navigation.

This PR overloads the "build" function of the GeometricSearchTrackerBuilder with another "build" function that builds the tracker and the MTD and arranges all the DetLayers properly. The PR also proposes a new TrackerMTDRecoGeometryESProducer invoking this alternative build function.

It should be noted that this producer is never used in the code so this development will not produce any change in the reconstruction, but it will help us to continue doing our developments to fully integrate the MTD in the tracking.

The code can be tested by running the TkNavigation tester:

RecoTracker/TkNavigation/test/NavigationSchoolAnalyzer.cc

by replacing the TrackerRecoGeometryESProducer by the TrackerMTDRecoGeometryESProducer in the configuration file:

RecoTracker/TkNavigation/test/NavigationSchoolAnalyzer_cfg.py

The ideas of this PR were discussed already some time ago in several meetings:

https://indico.cern.ch/event/923095/contributions/3878403/attachments/2047579/3431258/RECO-Talk-For-MTDDPGMay29.pdf

A github issue was also opened to discuss about this:

#29267

Thanks!

cmsbuild · 2021-03-25T16:25:50Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33272/21768

This PR adds an extra 40KB 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-33272/21768/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33272/21768/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-03-25T16:42:15Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33272/21769

This PR adds an extra 40KB to repository

cmsbuild · 2021-03-25T16:42:51Z

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

It involves the following packages:

RecoMTD/DetLayers
RecoTracker/GeometryESProducer
RecoTracker/Record
RecoTracker/TkDetLayers
RecoTracker/TkNavigation

@perrotta, @kpedro88, @cmsbuild, @srimanob, @slava77, @jpata can you please review it and eventually sign? Thanks.
@fabiocos, @makortel, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @gpetruc, @ebrondol, @mtosi, @dgulhan this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

fabiocos · 2021-03-26T09:37:16Z

RecoTracker/TkNavigation/test/NavigationSchoolAnalyzer_cfg.py

@@ -7,7 +7,8 @@
 # process.load("Configuration.StandardSequences.Geometry_cff")
 #process.load('Configuration.StandardSequences.GeometryRecoDB_cff')
 #process.load('Configuration.Geometry.GeometryExtended2023D1Reco_cff')
-process.load('Configuration.Geometry.GeometryExtended2023D4Reco_cff')
+#process.load('Configuration.Geometry.GeometryExtended2023D4Reco_cff')


@parbol strictly speaking we have no more 2023 geometries, so these lines are useless (even if harmless)

Hi Fabio,

I have remove the reference to the 2023 geometries in a later commit.

fabiocos · 2021-03-26T09:39:28Z

please test

cmsbuild · 2021-03-26T10:06:51Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4c715d/13788/summary.html
COMMIT: 5003364
CMSSW: CMSSW_11_3_X_2021-03-25-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33272/13788/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-03-25-2300/src/RecoTracker/TkDetLayers/interface/GeometricSearchTracker.h:4,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-03-25-2300/src/TrackingTools/RecoGeometry/interface/GlobalDetLayerGeometry.h:11,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-03-25-2300/src/TrackingTools/RecoGeometry/src/GlobalDetLayerGeometry.cc:1:
/cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_3_X_2021-03-25-2300/src/FWCore/Utilities/interface/Exception.h: In instantiation of 'typename cms::detail::Desired::type>::value>::type& cms::operator<<(E&&, const T&) [with E = cms::Exception&; T = DetId; typename cms::detail::Desired::type>::value>::type = cms::Exception&]':
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-03-25-2300/src/TrackingTools/RecoGeometry/src/GlobalDetLayerGeometry.cc:15:51:   required from here
/cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_3_X_2021-03-25-2300/src/FWCore/Utilities/interface/Exception.h:203:12: error: ambiguous overload for 'operator<<' (operand types are 'std::ostringstream' {aka 'std::__cxx11::basic_ostringstream'} and 'const DetId')
  203 |     e.ost_ << stuff;
      |     ~~~~~~~^~~~~~~~
In file included from /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc900/cms/cmssw-patch/CMSSW_11_3_X_2021-03-25-2300/src/DataFormats/DetId/interface/DetId.h:5,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-03-25-2300/src/TrackingTools/RecoGeometry/interface/GlobalDetLayerGeometry.h:10,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-03-25-2300/src/TrackingTools/RecoGeometry/src/GlobalDetLayerGeometry.cc:1:

cmsbuild · 2021-03-27T17:17:40Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33272/21789

This PR adds an extra 20KB 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-33272/21789/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33272/21789/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-04-09T13:31:48Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33272/21987

This PR adds an extra 88KB to repository

cmsbuild · 2021-04-09T13:32:08Z

Pull request #33272 was updated. @perrotta, @kpedro88, @cmsbuild, @srimanob, @slava77, @jpata can you please check and sign again.

fabiocos · 2021-04-09T13:40:34Z

please test

cmsbuild · 2021-04-09T18:02:01Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4c715d/14137/summary.html
COMMIT: fe53a85
CMSSW: CMSSW_11_3_X_2021-04-08-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33272/14137/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2865506
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2865477
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

jfernan2 · 2021-04-10T06:57:52Z

+1

perrotta · 2021-04-12T08:32:01Z

+1

It allows MTD DetLayers into the GeometricSearchTracker, to be used in the tracker navigation
Not yet enabled in production workflows, as some additional development is needed
The NavigationSchoolAnalyzer test runs succesfully also with MTD (it does not check the propagation up to the reconstructed objects though)
As it was stated at the reco meeting on Apr 9, TRK POG approves the addition
Jenkins tests pass with no differences, as expected

perrotta · 2021-04-12T08:45:00Z

@parbol @fabiocos

While reviewing this code I noticed a few possible simplifications for RecoTracker/TkDetLayers/src/GeometricSearchTrackerBuilder.cc.

Let me list them here, in case you want to apply them in a possible follow up PR:

Remove unneeded includes of "Geometry/TrackerGeometryBuilder/interface/trackerHierarchy.h" and "DataFormats/Common/interface/Trie.h"
Range based loops for the current it1 and all it2 iterators would simplify the code and improve its readability
An else if can be used in all checks for positionBounds()->z(), to avoid repeating those checks when the first one is passed
Avoid redefining useBrothers at L172, as it was already identically defined at L46

srimanob · 2021-04-12T13:02:14Z

+Upgrade

cmsbuild · 2021-04-12T13:02:39Z

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 · 2021-04-12T14:24:36Z

+1

cmsbuild added this to the CMSSW_11_3_X milestone Mar 25, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Mar 25, 2021

cmsbuild added code-checks-rejected code-checks-pending and removed code-checks-pending code-checks-rejected labels Mar 25, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 25, 2021

fabiocos reviewed Mar 26, 2021

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Mar 26, 2021

cmsbuild added tests-rejected code-checks-pending tests-pending and removed tests-started code-checks-approved tests-rejected labels Mar 26, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 27, 2021

Back to using get instead of get handle

fe53a85

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 9, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 9, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 9, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 9, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Apr 12, 2021

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Apr 12, 2021

cmsbuild added orp-approved and removed orp-pending labels Apr 12, 2021

cmsbuild merged commit d13b961 into cms-sw:master Apr 12, 2021

cmsbuild mentioned this pull request Apr 12, 2021

Make the the size of the binner (HistoContainer) settable at run time. The total number of Pixel Clusters is not a limit anymore on GPU #33371

Merged

perrotta mentioned this pull request Apr 13, 2021

Modernize code and simplify GeometricSearchTrackerBuilder #33420

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addition of the MTD to the tracker Navigation #33272

Addition of the MTD to the tracker Navigation #33272

parbol commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

fabiocos Mar 26, 2021

parbol Apr 5, 2021

fabiocos commented Mar 26, 2021

cmsbuild commented Mar 26, 2021

cmsbuild commented Mar 27, 2021

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

fabiocos commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

jfernan2 commented Apr 10, 2021

perrotta commented Apr 12, 2021

perrotta commented Apr 12, 2021 •

edited

Loading

srimanob commented Apr 12, 2021

cmsbuild commented Apr 12, 2021

qliphy commented Apr 12, 2021

Addition of the MTD to the tracker Navigation #33272

Addition of the MTD to the tracker Navigation #33272

Conversation

parbol commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

cmsbuild commented Mar 25, 2021

fabiocos Mar 26, 2021

Choose a reason for hiding this comment

parbol Apr 5, 2021

Choose a reason for hiding this comment

fabiocos commented Mar 26, 2021

cmsbuild commented Mar 26, 2021

Build

cmsbuild commented Mar 27, 2021

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

fabiocos commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

Comparison Summary

jfernan2 commented Apr 10, 2021

perrotta commented Apr 12, 2021

perrotta commented Apr 12, 2021 • edited Loading

srimanob commented Apr 12, 2021

cmsbuild commented Apr 12, 2021

qliphy commented Apr 12, 2021

perrotta commented Apr 12, 2021 •

edited

Loading