Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrating VectorHits reconstruction for the Phase2 OT - reprise #31314

Merged
merged 115 commits into from Oct 21, 2020

Conversation

JanFSchulte
Copy link
Contributor

@JanFSchulte JanFSchulte commented Sep 1, 2020

This is a renewed attempt to integrate the Vector Hits reconstruction in CMSSW. This is a continuation of PR #28101 (#28101), which was closed last year to allow for time to restructure the code.

In particular, the code has been refactored to no longer have an EDProducer as the product of an ESProducer. As before, this PR only integrates the reconstruction code but leaves it deactivated. When activated, this PR adds a new tracking iteration "pixelLess" to the phase 2 tracking targeting displaced tracks.

The code changes of this PR have been tested on 100 Events of the 27434.1 workflow in a recent IB. As expected, the tracking results are identical as the Vector Hits are inactive. This can be seen in the validation results here: https://jschulte.web.cern.ch/jschulte/VH_11_2_X/plots/plots_summary.html

The integration strategy for the VectorHits was last discussed at a TRK POG meeting last year: https://indico.cern.ch/event/847559/contributions/3572893/attachments/1914802/3165228/VectorHits_status_sept25.pdf.
However, the statements in those slides are still valid. A new assessment of the performance of the Vector Hits will be provided with the follow-up PR to activate them.

PR validation:

  • Compiliation after checking out all dependencies was sucessful

  • Passes runTheMatrix.py -l limited -i all --ibeos without errors


Final performance after code review, tested on 100 ttbar events at PU 200:

  • All tracks:
    Efficiency: 0.9122 -> 0.9292
    Fake rate: 0.1240 -> 0.0324
    Duplicate rate: 0.0045 -> 0.0064
  • High purity tracks:
    Efficiency: 0.9054 -> 0.9204
    Fake rate: 0.0549 -> 0.0153
    Duplicate rate: 0.0038 -> 0.0048

Efficiency gain is mostly located at low pT and efficiencies converge at around 10 GeV
Low pT fake rate is significantly reduced
image

PR significantly improves sensitivity to tracks with moderate displacement
image

With the current implementation this comes at a timing cost of about 20% for the tracking-only RECO step
image
Here the pixelLess iteration accounts for 40% of the total pattern recognition time, which can be improved with improvements to the seeding.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31314/18061

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31314/18062

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

A new Pull Request was created by @JanFSchulte (Jan-Frederik Schulte) for master.

It involves the following packages:

CommonTools/RecoAlgos
DataFormats/TrackerRecHit2D
Geometry/TrackerGeometryBuilder
RecoLocalTracker/Configuration
RecoLocalTracker/Records
RecoLocalTracker/SiPhase2VectorHitBuilder
RecoLocalTracker/SubCollectionProducers
RecoTracker/CkfPattern
RecoTracker/FinalTrackSelectors
RecoTracker/GeometryESProducer
RecoTracker/MeasurementDet
RecoTracker/TkDetLayers
RecoTracker/TkSeedGenerator
RecoTracker/TkSeedingLayers
RecoTracker/TransientTrackingRecHit
SimTracker/TrackAssociatorProducers

The following packages do not have a category, yet:

RecoLocalTracker/SiPhase2VectorHitBuilder
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@perrotta, @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @slava77, @jpata can you please review it and eventually sign? Thanks.
@echabert, @rappoccio, @felicepantaleo, @pieterdavid, @robervalwalsh, @mschrode, @abbiendi, @mmusich, @venturia, @makortel, @threus, @JanFSchulte, @jhgoh, @dgulhan, @jdolen, @yduhm, @GiacomoSguazzoni, @rovere, @VinInn, @gkasieczka, @hatakeyamak, @alesaggio, @ebrondol, @mtosi, @fabiocos, @clelange, @riga, @gbenelli, @ahinzmann, @gpetruc 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

@slava77
Copy link
Contributor

slava77 commented Sep 1, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

+1
Tested at: f155383
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7ffe9a/9039/summary.html
CMSSW: CMSSW_11_2_X_2020-09-01-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 1, 2020

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2609667
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2609633
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 10224.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 149 log files, 22 edm output root files, 35 DQM output files

@slava77
Copy link
Contributor

slava77 commented Oct 15, 2020

+1

for #31314 53aad7e

  • code changes are in line with the PR description and the follow up review: to integrate phase-2 outer tracker pt-module hit pairs as VectorHit (optionally enabled with a processModifier vectorHits)
  • jenkins tests pass and comparisons with the baseline show no differences; the profiler reports for the default setup show no relevant differences.

Links to trackinOnly tests with PU200 D49 setup with vectorHits enabled for the most recent tests are available in #31314 (comment)

Due to the length of the PR thread, it is not practical to look up some of the performance plots or to summarize what is in the final version of the PR.
@JanFSchulte please update the PR description so that it reflects the most recent updates
and perhaps also include the total time per event spent in tracking in the baseline and the latest variant of the PR using vectorHits for future reference

The longer term points discussed earlier are for some future time

  • about using looser cuts for the hits used in tracking (after seeding), instead of all combinations
  • see if something better can be done to avoid direct copies of hits (perhaps the point above will eliminate the need for this optimization)

@JanFSchulte
Copy link
Contributor Author

@slava77 I have updated the PR description with the required information.

@silviodonato
Copy link
Contributor

+operations

@chayanit
Copy link

+1

@kpedro88
Copy link
Contributor

+upgrade

@silviodonato
Copy link
Contributor

kind reminder @cms-sw/geometry-l2 @cms-sw/simulation-l2

@slava77
Copy link
Contributor

slava77 commented Oct 20, 2020

kind reminder @cms-sw/geometry-l2 @cms-sw/simulation-l2

It would be good to have this PR in pre8.
Is there a time estimate to get the geometry and simulation signatures?

@silviodonato
Copy link
Contributor

urgent
for pre8

@cvuosalo
Copy link
Contributor

+1

@silviodonato
Copy link
Contributor

merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet