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

Enable pixel templates for phase1 pixel #14159

Merged
merged 1 commit into from Apr 29, 2016

Conversation

makortel
Copy link
Contributor

The title says it all. The templates are already part of the GTs, so enabling them is just a matter of removing all customizations changing TransientTrackingRecHitBuilder to "WithTrackAngle".

Tested in 8_1_0_pre2, small changes expected in tracking and downstream (most notably dxy and dz resolutions improve). I have the usual set of MTV plots with 1000 TTbar+35PU events here (RAW from 810pre1, ideal GT) https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_8_1_0_pre2_template

@rovere @VinInn @dkotlins

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

RecoMuon/Configuration
RecoMuon/GlobalTrackingTools
RecoMuon/MuonIdentification
RecoMuon/TrackingTools
RecoParticleFlow/PFTracking
RecoTracker/FinalTrackSelectors
RecoTracker/SpecialSeedGenerators
RecoTracker/TrackProducer

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @mmarionncern, @battibass, @abbiendi, @GiacomoSguazzoni, @rafaellopesdesa, @jhgoh, @lgray, @bellan, @mschrode, @rovere, @amagitte, @gpetruc, @cerati, @VinInn, @trocino, @dgulhan, @bachtis, @rociovilar this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@slava77
Copy link
Contributor

slava77 commented Apr 20, 2016

@cmsbuild please test
not sure how broken is the jenkins baseline. So, tests may need to be rerun

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/12514/console

@cmsbuild
Copy link
Contributor

-1

Tested at: ef463c1

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14159/12514/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
140.53 step2

runTheMatrix-results/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/step2_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI.log
1000.0 step2
runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log
1001.0 step2
runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4.log
1003.0 step2
runTheMatrix-results/1003.0_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM/step2_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM.log

@slava77
Copy link
Contributor

slava77 commented Apr 21, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/12535/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Apr 22, 2016

@cmsbuild please test
comparisons didn't make it through jenkins

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/12596/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Apr 26, 2016

Here are some random plots from CMSSW_8_1_0_pre3 matrix workflows with largish differences
In QCD600-800 (wf10026)
all_sign691-histatvsorig-histat_qcd600to800in13tev2017wf10026p0c_recocandidateedmptrsrecojettaginforecoiptaginfos_pfimpactparametertaginfos__reco_obj_m_prob2d
But there seems to be no significant impact on the ROC plot
wf10026_btag_roc_2d

ttbar with pileup
wf10224_general_chi2ndof

This compared to no change observed with the design GT
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_8_1_0_pre2_template/ttbar_pu_ootb/tuning.pdf

and a few more in the general tracks distributions for ttbar with PU (I couldn't find the same in the MTV plots, may have missed though).
wf10224_general_losthitseta
wf10224_general_dzpulleta

I'm not sure if these are indications of something incorrect in the current application of templates in phase1 pixel.

@dkotlins
Copy link
Contributor

We still do not have the final (correct) geometry for the endcap part of phase1 pixels.
This might affect the resolution when using templates.
The generic reco is more robust in this respect.
So possibly such problems would still need to be fixed.
Danek

On 26 Apr 2016, at 09:50, Slava Krutelyov notifications@github.com wrote:

Here are some random plots from CMSSW_8_1_0_pre3 matrix workflows with largish differences
In QCD600-800 (wf10026)

But there seems to be no significant impact on the ROC plot

ttbar with pileup

This compared to no change observed with the design GT
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_8_1_0_pre2_template/ttbar_pu_ootb/tuning.pdf

and a few more in the general tracks distributions for ttbar with PU (I couldn't find the same in the MTV plots, may have missed though).

I'm not sure if these are indications of something incorrect in the current application of templates in phase1 pixel.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@slava77
Copy link
Contributor

slava77 commented Apr 26, 2016

Also, looking at timing on 100 events in 10224 (ttbar PU 35, 2017 scenario):
skipping validation modules, the following appear to change timing quite a bit (the last few are probably in the noise, but the rest is quite significant)

   +0.620969      +0.12%       118.38 ms/ev ->       224.98 ms/ev lowPtQuadStepTracks
   +0.596382      +0.11%       112.73 ms/ev ->       208.52 ms/ev lowPtTripletStepTracks
   +0.591281      +0.13%       132.68 ms/ev ->       244.06 ms/ev detachedQuadStepTracks
   +0.552268      +0.13%       143.82 ms/ev ->       253.54 ms/ev highPtTripletStepTracks
   +0.545402      +0.21%       243.83 ms/ev ->       426.68 ms/ev initialStepTracks
   +0.545364      +0.21%       242.60 ms/ev ->       424.50 ms/ev initialStepTracksPreSplitting
   +0.440055      +0.01%        16.46 ms/ev ->        25.74 ms/ev mergedDuplicateTracks
   +0.084902      +0.01%       130.66 ms/ev ->       142.24 ms/ev pixelLessStepTracks
   +0.055614      +0.01%        79.66 ms/ev ->        84.21 ms/ev trackerDrivenElectronSeeds
   -0.052607      -0.00%        63.28 ms/ev ->        60.04 ms/ev tobTecStepSeedsPair
Totals in printed: 1284.1 -> 2094.51

Is this expected?

@dkotlins
Copy link
Contributor

Templates are slower than generic, this is why at HLT only generic is used.
But I do not remember exactly how much slower?
Maybe we have somewhere a comparison for phase0?
D.

On 26 Apr 2016, at 10:55, Slava Krutelyov notifications@github.com wrote:

Also, looking at timing on 100 events in 10224 (ttbar PU 35, 2017 scenario):
skipping validation modules, the following appear to change timing quite a bit (the last few are probably in the noise, but the rest is quite significant)

+0.620969 +0.12% 118.38 ms/ev -> 224.98 ms/ev lowPtQuadStepTracks
+0.596382 +0.11% 112.73 ms/ev -> 208.52 ms/ev lowPtTripletStepTracks
+0.591281 +0.13% 132.68 ms/ev -> 244.06 ms/ev detachedQuadStepTracks
+0.552268 +0.13% 143.82 ms/ev -> 253.54 ms/ev highPtTripletStepTracks
+0.545402 +0.21% 243.83 ms/ev -> 426.68 ms/ev initialStepTracks
+0.545364 +0.21% 242.60 ms/ev -> 424.50 ms/ev initialStepTracksPreSplitting
+0.440055 +0.01% 16.46 ms/ev -> 25.74 ms/ev mergedDuplicateTracks
+0.084902 +0.01% 130.66 ms/ev -> 142.24 ms/ev pixelLessStepTracks
+0.055614 +0.01% 79.66 ms/ev -> 84.21 ms/ev trackerDrivenElectronSeeds
-0.052607 -0.00% 63.28 ms/ev -> 60.04 ms/ev tobTecStepSeedsPair
Totals in printed: 1284.1 -> 2094.51

Is this expected?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@makortel
Copy link
Contributor Author

@slava77 Your observation is consistent with my tests
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_8_1_0_pre2_template/ttbar_pu_timing/iterations.pdf
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_8_1_0_pre2_template/ttbar_pu_timing/summary.pdf
i.e. track fitting becomes slower (~50 % in my test). On the other hand, pattern recognition becomes ~7 % faster, and the overall slowdown of iterative tracking is ~4 %.

@slava77
Copy link
Contributor

slava77 commented Apr 26, 2016

@makortel
From your plots (summary.pdf)
Building time is reduced by 4% in absolute time in ms.
If I'm not mistaken, the "pattern recognition becomes ~7 % faster" is relative to the total tracking time.

I, unfortunately don't see substantial changes in other modules; maybe it's the 100 event statistics that's less precise, but it could be something due to a different GT used in tests and slightly different timing there.

   -0.012681      -0.01%       591.64 ms/ev ->       584.18 ms/ev pixelLessStepSeeds
   +0.008604      +0.01%      1400.30 ms/ev ->      1412.40 ms/ev detachedQuadStepSeeds
   +0.024100      +0.02%       587.06 ms/ev ->       601.38 ms/ev lowPtTripletStepTrackCandidates
   +0.032145      +0.03%       759.42 ms/ev ->       784.23 ms/ev highPtTripletStepTrackCandidates

Anyways, it looks like the timing increase is expected.

The question is if the switch to templates is done too early, since it apparently enhances the issues apparently connected to the endcap geometry
(maybe it would be clear with some fraction of plots showing things going better overall).

@makortel
Copy link
Contributor Author

@slava77 Right, I read the plot sloppily, forget the "~7 % faster". The building time (what I called "pattern recognition") is indeed ~4 % faster. The reason for the different results could indeed lie in the GT as I used the design one.

The clearest improvement is in the dxy and dz resolution for pT > ~0.8 GeV https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_8_1_0_pre2_template/ttbar_pu_ootb/resolutionsPt.pdf

@slava77
Copy link
Contributor

slava77 commented Apr 26, 2016

@makortel
Thanks, the ratio plots are useful.

@slava77
Copy link
Contributor

slava77 commented Apr 26, 2016

+1

for #14159 ef463c1

  • code changes are in line with the description
  • jenkins tests pass and comparisons with baseline show no differences in workflows up to run2 2016 and there are small differences in 2017 workflows as expected
  • local tests in 810pre3 of 2017 workflows show some degradations more noticeable in pixel endcap (somewhat expected, but they should go away after updates to the pixel geometry);

@cmsbuild
Copy link
Contributor

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

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 1d52eaa into cms-sw:CMSSW_8_1_X Apr 29, 2016
@makortel makortel deleted the phase1_enable_template branch April 19, 2017 09:47
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

5 participants