Cleanup and modernize SeedingLayerSetsBuilder #21138

makortel · 2017-11-02T15:51:31Z

This PR removes unused and obsolete code related to SeedingLayerSetsBuilder:

ctfseeding::SeedingLayer class altogether (made obsolete by Seeding layers ESP to EDP migration #2378)
- TrackingRegion::hits(const EventSetup&, const SeedingLayer *)
- The only still-useful piece of SeedingLayer was SeedingLayer::Side enumeration, which I moved to DataFormats/TrackerCommon/interface/TrackerDetSide.h as TrackerDetSide enumeration (still "old-style" enum as the numerical values are heavily used)

and does some modernization

use unique_ptr instead of shared_ptr (since it's enough)
add fillDescriptions (even if it is incomplete)
- as a consequence some strip pair layer lists are updated to use the same layer naming convention as in iterative tracking configuration
improve the relation between SeedingLayerSetsBuilder and SeedingLayerEDProducer

The last two points are also a preparatory work for the upcoming "automated mitigation for pixel inactive areas in pixel pair iteration".

Tested in 9_4_0_pre3, no changes expected in results except for MTV seeding layer histograms for conversion seeds

for some layers TIBn/TOBn move to MTIBn/MTOBn
TECn get split to TECn and MTECn

@VinInn

…nused and obsolete

In the near future I plan to use SeedingLayerSetsBuilder::hits() elsewhere than SeedingLayersEDProducer, and for that I want the construction of SeedingLayerSetsHits to be solely on the responsibility of SeedingLayerSetsBuilder.

…erDetSide

Still with setAllowAnything(), but better than nothing.

Towards fillDescriptions in SeedingLayersEDProducer, makes it easier if there are no TID[1-4].

cmsbuild · 2017-11-02T15:51:58Z

The code-checks are being triggered in jenkins.

cmsbuild · 2017-11-02T15:54:34Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21138/1773

cmsbuild · 2017-11-02T15:54:59Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

DataFormats/TrackerCommon
RecoMuon/TrackerSeedGenerator
RecoPixelVertexing/PixelLowPtUtilities
RecoPixelVertexing/PixelTrackFitting
RecoPixelVertexing/PixelTriplets
RecoTracker/Configuration
RecoTracker/ConversionSeedGenerators
RecoTracker/SpecialSeedGenerators
RecoTracker/TkSeedingLayers
RecoTracker/TkTrackingRegions
TrackingTools/TransientTrackingRecHit
Validation/RecoTrack

@perrotta, @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @slava77, @vanbesien can you please review it and eventually sign? Thanks.
@ghellwig, @trocino, @calderona, @felicepantaleo, @abbiendi, @GiacomoSguazzoni, @rovere, @VinInn, @echapon, @bellan, @HuguesBrun, @jhgoh, @wmtford, @mschrode, @gpetruc, @ebrondol, @rociovilar, @mtosi, @dgulhan, @bachtis, @battibass this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

makortel · 2017-11-02T16:04:46Z

@cmsbuild, please test

cmsbuild · 2017-11-02T16:05:19Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/24141/console Started: 2017/11/02 17:05

cmsbuild · 2017-11-10T13:18:27Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21138/1924

cmsbuild · 2017-11-10T13:18:54Z

Pull request #21138 was updated. @perrotta, @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @jfernan2, @slava77, @vanbesien can you please check and sign again.

makortel · 2017-11-10T13:20:14Z

@cmsbuild, please test

cmsbuild · 2017-11-10T13:20:44Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/24345/console Started: 2017/11/10 15:19

cmsbuild · 2017-11-10T17:07:29Z

+1
Tested at: 733226a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21138/24345/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
3eecb4f
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21138/24345/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21138/24345/git-merge-result

cmsbuild · 2017-11-10T17:07:32Z

Comparison job queued.

cmsbuild · 2017-11-10T19:40:47Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 27
DQMHistoTests: Total histograms compared: 2832699
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 101
DQMHistoTests: Total successes: 2832419
DQMHistoTests: Total skipped: 178
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 10.04 KiB( 23 files compared)
Checked 111 log files, 8 edm output root files, 27 DQM output files

slava77 · 2017-11-11T11:16:50Z

+1

for #21138 733226a

changes are in line with the PR description. No changes in reco quantities. Some seedingLayer index reshuffling (renaming and split of TEC to TEC+MTEC) in MTV conversion plots.
jenkins tests pass and comparisons with baseline show only expected differences [ignoring 8 reco diffs in logErrorHarvester in PU workflows apparently due to file re-open]

Here is an example from 10224 concersionStep
top is baseline, bottom is with this PR

the "TEC+MTEC" adds up to what was there in TEC before.

dmitrijus · 2017-11-14T15:30:10Z

+1

cmsbuild · 2017-11-14T15:31:03Z

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 (and backports should be raised in the release meeting by the corresponding L2)

davidlange6 · 2017-11-14T19:54:16Z

+1

makortel added 15 commits November 1, 2017 21:55

Remove SeedingLayerSetsBuilder::layers() as unused and obsolete

583ab90

Remove CosmicTrackingRegion::hits(.., ctfseeding::SeedingLayer*) as u…

87deccb

…nused and obsolete

Remove unnecessary includes of SeedingLayer.h

3cbaeaf

Remove all functionality from SeedingLayer as unused and obsolete

0d873a8

Remove forward declaration

a7f9c13

Minor modernization

e6b9ddf

Deliver also DetLayer vector via pointer for consistency

551574a

Remove SeedingLayer.h as unnecessary, move enum to more general Track…

c6dcf57

…erDetSide

Remove unnecessary mutable

f3de976

Use unique_ptr

ebfb745

Simplify updating from EventSetup

57af369

Move SeedingLayersEDProducer to fillDescriptions

935b148

Still with setAllowAnything(), but better than nothing.

Unify layer naming with iterative tracking and reduce copy-paste

d52a98b

Towards fillDescriptions in SeedingLayersEDProducer, makes it easier if there are no TID[1-4].

Call updateEventSetup inside hits() in SeedingLayerSetsBuilder

2a09975

cmsbuild added this to the CMSSW_10_0_X milestone Nov 2, 2017

cmsbuild added code-checks-pending comparison-pending dqm-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Nov 2, 2017

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 2, 2017

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 10, 2017

cmsbuild added tests-started and removed tests-pending labels Nov 10, 2017

cmsbuild added tests-approved and removed tests-started labels Nov 10, 2017

cmsbuild added comparison-available and removed comparison-pending labels Nov 10, 2017

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Nov 11, 2017

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Nov 14, 2017

cmsbuild added orp-approved and removed orp-pending labels Nov 14, 2017

cmsbuild merged commit 0b137f2 into cms-sw:master Nov 14, 2017

makortel deleted the cleanupSeedingLayerSetsBuilder branch February 12, 2018 13:04

makortel mentioned this pull request Sep 25, 2018

Backport of Phase1 Tracking in FastSim to 94X #24624

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup and modernize SeedingLayerSetsBuilder #21138

Cleanup and modernize SeedingLayerSetsBuilder #21138

makortel commented Nov 2, 2017 •

edited

cmsbuild commented Nov 2, 2017

cmsbuild commented Nov 2, 2017

cmsbuild commented Nov 2, 2017

makortel commented Nov 2, 2017

cmsbuild commented Nov 2, 2017 •

edited

cmsbuild commented Nov 10, 2017

cmsbuild commented Nov 10, 2017

makortel commented Nov 10, 2017

cmsbuild commented Nov 10, 2017 •

edited

cmsbuild commented Nov 10, 2017

cmsbuild commented Nov 10, 2017

cmsbuild commented Nov 10, 2017

slava77 commented Nov 11, 2017

dmitrijus commented Nov 14, 2017

cmsbuild commented Nov 14, 2017

davidlange6 commented Nov 14, 2017

Cleanup and modernize SeedingLayerSetsBuilder #21138

Cleanup and modernize SeedingLayerSetsBuilder #21138

Conversation

makortel commented Nov 2, 2017 • edited

cmsbuild commented Nov 2, 2017

cmsbuild commented Nov 2, 2017

cmsbuild commented Nov 2, 2017

makortel commented Nov 2, 2017

cmsbuild commented Nov 2, 2017 • edited

cmsbuild commented Nov 10, 2017

cmsbuild commented Nov 10, 2017

makortel commented Nov 10, 2017

cmsbuild commented Nov 10, 2017 • edited

cmsbuild commented Nov 10, 2017

cmsbuild commented Nov 10, 2017

cmsbuild commented Nov 10, 2017

slava77 commented Nov 11, 2017

dmitrijus commented Nov 14, 2017

cmsbuild commented Nov 14, 2017

davidlange6 commented Nov 14, 2017

makortel commented Nov 2, 2017 •

edited

cmsbuild commented Nov 2, 2017 •

edited

cmsbuild commented Nov 10, 2017 •

edited