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

Displace-tracking development in heavy ion tracking #21930

Merged
merged 19 commits into from Feb 16, 2018

Conversation

KongTu
Copy link
Contributor

@KongTu KongTu commented Jan 23, 2018

This displace-tracking development in heavy ion reconstruction is to find (like CMS standard pp tracking) the large impact parameter tracks for strange particles or heavy-flavor particles reconstruction.

This PR contains the following items:

  • Added the configurations for 3 iterations: "hiMixedTripletStep", "hiPixelLessStep", "hiTobTecStep". Modified on top of the default tracking algorithm from standard pp tracking.
  • The TrackingRegion parameters are modified to reduce the timing on these 3 iterations above, e.g., the minPt and originHalfLength.
  • A "scaling" function has been implemented (in this file GlobalTrackingRegionWithVerticesProducer.h), where according to the number of pixel clusters (mostly for more central heavy ion events), the TrackingRegion parameters, e.g., minPt, originRadius, originHalfLength, are scaled up (pt) or down (originRaidu or originHalfLength) linearly above a certain threshold (for example Npix=20000). Therefore the timing can be greatly reduced towards very central events.

Supporting presentations:

@abaty @mandrenguyen

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @KongTu (KongTu) for master.

It involves the following packages:

RecoHI/HiTracking
RecoTracker/TkTrackingRegions

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @felicepantaleo, @GiacomoSguazzoni, @jazzitup, @VinInn, @echapon, @mschrode, @yenjie, @rovere, @kurtejung, @gpetruc, @ebrondol, @mandrenguyen, @dgulhan, @yetkinyilmaz 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

@slava77
Copy link
Contributor

slava77 commented Jan 23, 2018

@KongTu

Timing study as a function of Npix, https://indico.cern.ch/event/699094/contributions/2867255/attachments/1587914/2511992/Timing_displacedTracking_Jan24.pdf

the plot has "blue" (this PR) both above and below the baseline.
Please clarify what is happening in the lower bins.

@slava77
Copy link
Contributor

slava77 commented Jan 23, 2018

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 23, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25581/console Started: 2018/01/23 20:18

@abaty
Copy link
Contributor

abaty commented Jan 23, 2018

@slava77 @KongTu

the plot has "blue" (this PR) both above and below the baseline.
Please clarify what is happening in the lower bins.

Each point here represents the processing time of a single event, so I suspect the difference seen in the lower bins is just a fluctuation in the processing time. The code here does not 'remove' any features for central events to make them run faster; it only adds tracking iterations for more peripheral events.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #21930 was updated. @perrotta, @cmsbuild, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Feb 15, 2018

@cmsbuild please test

@abaty
thank you for the quick updates

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 15, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26100/console Started: 2018/02/15 16:52

@cmsbuild
Copy link
Contributor

Comparison job queued.


mixedTripletStepSeedLayersA.layerList = cms.vstring('BPix1+BPix2+BPix3', 'BPix1+BPix2+FPix1_pos', 'BPix1+BPix2+FPix1_neg',
'BPix1+FPix1_pos+FPix2_pos', 'BPix1+FPix1_neg+FPix2_neg',
'BPix2+FPix1_pos+FPix2_pos', 'BPix2+FPix1_neg+FPix2_neg')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the modifications made here actually propagate to hiRegitMuMixedTripletStepSeedLayersA.
I'm not sure that this is desired.

Based on the phase-0 configuration (wf 140.53) this is the only place with such dependency.

An easy fix is to set the layerList in hiRegitMuMixedTripletStepSeedLayersA explicitly.
OTOH, perhaps it's OK to have a more covering layer list for hiRegitMuMixedTripletStepSeedLayersA

@KongTu @abaty please check

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slava77 I think having the extra layers in the list for the Regit step is fine; these steps tend to run quite quickly and produce a fairly small number of tracks. It is probably best for consistency to keep these two iterations using the same layer list.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 25 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2465360
  • DQMHistoTests: Total failures: 29
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2465162
  • DQMHistoTests: Total skipped: 169
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.07999999987 KiB( 22 files compared)
  • Checked 111 log files, 9 edm output root files, 27 DQM output files

@slava77
Copy link
Contributor

slava77 commented Feb 15, 2018

Based on wf 150.0 I see that

  • CPU time in reco increases by about 25% (from 11.4 to 14.2 s/ev)
    • most of the increase is in the new modules +2.7 s/ev
    • the rest is about 0.2 s/ev and it shows up in related modules (e.g. hiGeneralTracks or hiMuons1stStep, and particleFlowBlock);
  • RSS peak (based on MemoryCheck) goes up by about 100 MB. This looks ~OK and should likely be smaller on a job writing AOD

Physics performance roughly looks as expected:

  • there are about 20% more tracks
  • CS jets look somewhat similar
  • the efficiency increases similarly for HP and all tracks

wf150 0_tk_eff_pt

- fake rates are up for inclusive tracks (some cleanup is likely needed in a followup), while for HP tracks the fake rate is only about 10-20% (fractionally) higher

wf150 0_tk_fr_pt

- the duplicate rate is up significantly (is merging good enough or needs some reconfiguration eventually?)

wf150 0_tk_dup_pt

I'm ready to signoff pending the resolution of the hiRegitMuMixedTripletStepSeedLayersA configuration

@slava77
Copy link
Contributor

slava77 commented Feb 16, 2018

+1

for #21930 1b849c7

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor

+1

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

8 participants