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

Adjust seeding region rebuilding parameters for phase1 initialStep #17544

Merged
merged 1 commit into from Feb 21, 2017

Conversation

makortel
Copy link
Contributor

This PR reduces the high-eta duplicates as the seeding region is rebuilt more aggressively. Inspired by the phase2 configuration.

Here are MTV plots for phase1 default tracking in 9_0_0_pre4+#17537+#17511
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_9_0_0_pre4_phase1SeedRegionRebuild/

Tested in 9_0_0_pre4, expecting changes indicated above in phase1 workflows. No changes are expected in phase0/2.

@rovere @VinInn @felicepantaleo @ebrondol

Should reduce the high-eta duplicates as the seeding region rebuilding
is done more aggressively. Inspired by the phase2 configuration.
@makortel
Copy link
Contributor Author

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 17, 2017

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

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

RecoTracker/IterativeTracking

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @gpetruc, @dgulhan this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@makortel
Copy link
Contributor Author

Regarding the visible efficiency drop in highPurity
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_9_0_0_pre4_phase1SeedRegionRebuild/phase1_ttbar_pu35_highPurity/effandfakePtEtaPhi.pdf
the efficiency of generalTracks stays the same
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_9_0_0_pre4_phase1SeedRegionRebuild/phase1_ttbar_pu35_ootb/effandfakePtEtaPhi.pdf
so the decrease is an effect of the HP track selection. Therefore we plan to re-train the MVA (at least for initialStep, for 91X).

Wrt. 900pre4 (i.e. the combined effect of #17537+#17511+this PR) the HP efficiency mostly improves
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_9_0_0_pre4_phase1SeedRegionRebuild2/phase1_ttbar_pu35_highPurity/effandfakePtEtaPhi.pdf

@slava77
Copy link
Contributor

slava77 commented Feb 21, 2017

In ttbar PU35 (wf 10224) with CMSSW_9_0_X_2017-02-16-1100 as a baseline

generalTracks count is down by ~0.15%
the shorter (nhits~<12) tracks are longer
all_sign863vsorig_ttbar13tevpu2017wf10224p0c_recotracks_generaltracks__reco_obj_found

pf charged hadron yield is up by ~0.15% with most noticeable increase in |eta|>2.5
all_sign863vsorig_ttbar13tevpu2017wf10224p0c_recopfcandidates_particleflow__reco_obj_eta43

So, incrementally, this is an improvement, although it apparently goes worse on top of the updated MVAs mentioned earlier https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_9_0_0_pre4_phase1SeedRegionRebuild/phase1_ttbar_pu35_highPurity/effandfakePtEtaPhi.pdf (although there may be it's a matter of choosing a different baseline)

Combined with the already MVA PR, there is still fairly significant increment in yields at high eta on top of the baseline
all_sign863avsorig_ttbar13tevpu2017wf10224p0c_recopfcandidates_particleflow__reco_obj_eta43
wf10224_sign863avsorig_hp_eff_eta

and indeed, there is a moderate decrease in high-eta yields incrementally on top of the MVA PR ( #17537)
all_sign863avssign861_ttbar13tevpu2017wf10224p0c_recopfcandidates_particleflow__reco_obj_eta43
wf10224_sign863avssign861_hp_eff_eta

The last plot has efficiency loss much smaller than in positive eta (and somewhat comparable to the negative eta) of
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_9_0_0_pre4_phase1SeedRegionRebuild/phase1_ttbar_pu35_highPurity/effandfakePtEtaPhi.pdf
wf10224matti_incr_hp_eff_dup_v_eta
@makortel I didn't include #17511 in this incremental comparison .. is there additional destructive interference in positive eta with the MVA selection from it (#17511) and this PR?

I suppose, the arguments for having this PR included now is that it should be considered in a combination of the features that went in for tracking since pre4.
By itself it's rather questionable, driven by the HP efficiency loss (since HP defines what goes to PF).

@makortel
Copy link
Contributor Author

@slava77 #17511 should be rather independent of MVA selection, as the duplicate merging is done after all iterations. This PR, on the other hand, "interferes" with the updated MVA such that the MVA needs to be retrained.

@slava77
Copy link
Contributor

slava77 commented Feb 21, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Feb 21, 2017

+1

for #17544 e6685f6

  • changes in initialStepTrajectoryBuilder in order to reduce high eta duplicates, as described; comes with some destructive interference incrementally on top of Retrained track selection MVA for phase1 #17537, but still is an improvement in combination with Retrained track selection MVA for phase1 #17537 (already merged).
  • jenkins tests pass and comparisons with baseline show differences only in phase-1 workflows with small changes starting from tracks and propagating downstream
  • local tests with extended statistics confirm observations in the supplied MTV tracking plots; the changes in tracks passing high-purity show up as slight changes in the pf charged hadrons most noticeable in |eta|>2.5 on top of the Retrained track selection MVA for phase1 #17537; further downstream performance appears to be essentially not affected. Signing off in view of getting rid of a significant fraction of short tracks and a combined performance improvement with the already merged Retrained track selection MVA for phase1 #17537 .
    • timing performance in 10224 appears to be somewhat unchanged, there maybe around 1% improvement, but since there is no clear localization across heavy modules, this is not significant.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
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

4 participants