Reducing duplicates in Phase2 tracking reconstruction (with some minor effects on phase0/1 tracking) #14994

ebrondol · 2016-06-27T14:41:21Z

With this PR, I am importing the PR#3051 done in SLHC by @cerati .
A first comparison can be seen here.
Since this part is touching the run2 reconstruction, I have already run the most important WFs and check that everything produce the same results. Still, a more detail test analysis is for sure needed.

@VinInn @rovere

cmsbuild · 2016-06-27T14:41:42Z

A new Pull Request was created by @ebrondol for CMSSW_8_1_X.

It involves the following packages:

RecoTracker/CkfPattern

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @gpetruc, @dgulhan 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 · 2016-06-27T14:43:44Z

RecoTracker/CkfPattern/plugins/GroupedCkfTrajectoryBuilder.cc

+    //  rebuiltTrajectories.push_back(std::move(*it));
+    //  LogDebug("CkfPattern")<< "RebuildSeedingRegion skipped as in-out trajectory does not exceed seed size.";
+    //  continue;
+    //}


shouldn't this and the other one be made configurable?
That's unless we never ever make it into this branch in current tracking.

As far as I searched, this change was never made in the running tracking.

If this code is not needed, please remove.
If for some reason it's needed here commented out, please add a clear comment why.
The same for a change in line 1021

I do not have a strong opinion on that, @VinInn or @rovere do we want to keep it for a future reference or we can get rid of it? @cerati did you keep it in your PR at that time for a specific reason?

slava77 · 2016-06-27T14:44:10Z

@cmsbuild please test

cmsbuild · 2016-06-27T14:44:29Z

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

cmsbuild · 2016-06-27T15:52:51Z

-1

Tested at: c8beafd

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14994/13697/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:
1001.0 step1

DAS Error

slava77 · 2016-06-27T15:54:12Z

@cmsbuild please test
hopefully not DAS problems this time

cmsbuild · 2016-06-27T15:54:37Z

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

cmsbuild · 2016-06-27T17:15:41Z

+1
Tested at: c8beafd
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14994/13700/summary.html

cmsbuild · 2016-06-27T18:09:40Z

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

There are some workflows for which there are errors in the baseline:
5.1 step 2
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

slava77 · 2016-06-28T16:07:38Z

Jenkins shows some differences from this PR.
We need standard MTV plots if there are changes.

If this is specific for Phase2, please make the logic configurable.
If this is generic to all tracking, please edit the PR description and title.

makortel · 2016-06-28T19:38:40Z

FWIW, I had already the MTV plots for phase1 (1000 ttbar+PU events)
https://mkortela.web.cern.ch/mkortela/tracking/validation/CMSSW_8_1_0_pre5_v17/
(blue is the phase1 tracking as in 810pre7, and red is with the changes in this PR in addition; although run in 810pre5 with 810pre4 RAW). In short, there are tiny changes that look mostly be caused changes in track pT.

slava77 · 2016-07-04T16:27:49Z

@ebrondol
I didn't get to this yet. Likely, tomorrow.

slava77 · 2016-07-05T17:18:50Z

@ebrondol
please improve the title of the PR.
It should reflect that run2/current tracking behavior changes as well (may add "minor effects on run1/2 tracking")

slava77 · 2016-07-06T16:43:02Z

As expected, there are significant changes mainly in 2023 workflows.

most typical change in run2 workflow is a slight change in the algorithm or quality mask (in the workflows with a change a pf charged hadron would be lost)
e.g. 200 events in doubleMu 2016B

2023 ttbar 10624:

less tracks from higher iterations

similarly in 2023 QCD 600-800 (wf 10626).

Beyond this, I'm relying on the MTV plots posted earlier at PR submission
#14994 (comment)

Other than the issue in
#14994 (comment)
this looks good to go

cmsbuild · 2016-07-07T09:52:53Z

Pull request #14994 was updated. @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please check and sign again.

ebrondol · 2016-07-07T09:53:10Z

Removed.

VinInn · 2016-07-07T09:53:35Z

@cmsbuild , please test

cmsbuild · 2016-07-07T09:53:56Z

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

cmsbuild · 2016-07-07T11:15:44Z

+1
Tested at: 5fcf103
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14994/13928/summary.html

cmsbuild · 2016-07-07T13:32:31Z

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

slava77 · 2016-07-07T14:33:03Z

+1

for #14994 5fcf103

code change is OK
jenkins tests (now include 2023 workflows!) confirm expected behavior
results from tests with more events were posted earlier Reducing duplicates in Phase2 tracking reconstruction (with some minor effects on phase0/1 tracking) #14994 (comment) (the last commit removed only the initially commented out lines)

cmsbuild · 2016-07-07T14:36:40Z

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

ebrondol · 2016-07-07T14:37:41Z

@kpedro88 just for your info

importing SLHC PR3051

c8beafd

cmsbuild added this to the Next CMSSW_8_1_X milestone Jun 27, 2016

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending comparison-pending labels Jun 27, 2016

slava77 reviewed Jun 27, 2016
View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Jun 27, 2016

cmsbuild added tests-rejected and removed tests-started labels Jun 27, 2016

cmsbuild added tests-pending and removed tests-rejected labels Jun 27, 2016

cmsbuild added tests-started and removed tests-pending labels Jun 27, 2016

cmsbuild added tests-approved and removed tests-started labels Jun 27, 2016

cmsbuild added comparison-available and removed comparison-pending labels Jun 27, 2016

ebrondol changed the title ~~Reducing duplicates in Phase2 tracking reconstruction~~ Reducing duplicates in Phase2 tracking reconstruction (minor effects on phase0/1 tracking) Jul 5, 2016

ebrondol changed the title ~~Reducing duplicates in Phase2 tracking reconstruction (minor effects on phase0/1 tracking)~~ Reducing duplicates in Phase2 tracking reconstruction (with some minor effects on phase0/1 tracking) Jul 5, 2016

removing

5fcf103

cmsbuild added tests-pending comparison-pending and removed comparison-available tests-approved labels Jul 7, 2016

cmsbuild added tests-started and removed tests-pending labels Jul 7, 2016

cmsbuild added tests-approved and removed tests-started labels Jul 7, 2016

cmsbuild added comparison-available and removed comparison-pending labels Jul 7, 2016

cmsbuild added reconstruction-approved fully-signed and removed pending-signatures reconstruction-pending labels Jul 7, 2016

davidlange6 merged commit 042435f into cms-sw:CMSSW_8_1_X Jul 9, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reducing duplicates in Phase2 tracking reconstruction (with some minor effects on phase0/1 tracking) #14994

Reducing duplicates in Phase2 tracking reconstruction (with some minor effects on phase0/1 tracking) #14994

ebrondol commented Jun 27, 2016

cmsbuild commented Jun 27, 2016

slava77 Jun 27, 2016

ebrondol Jun 27, 2016

slava77 Jul 5, 2016

ebrondol Jul 5, 2016

VinInn Jul 7, 2016

slava77 commented Jun 27, 2016

cmsbuild commented Jun 27, 2016 •

edited

cmsbuild commented Jun 27, 2016

slava77 commented Jun 27, 2016

cmsbuild commented Jun 27, 2016 •

edited

cmsbuild commented Jun 27, 2016

cmsbuild commented Jun 27, 2016

slava77 commented Jun 28, 2016

makortel commented Jun 28, 2016

slava77 commented Jul 4, 2016

slava77 commented Jul 5, 2016

slava77 commented Jul 6, 2016

cmsbuild commented Jul 7, 2016

ebrondol commented Jul 7, 2016

VinInn commented Jul 7, 2016

cmsbuild commented Jul 7, 2016 •

edited

cmsbuild commented Jul 7, 2016

cmsbuild commented Jul 7, 2016

slava77 commented Jul 7, 2016

cmsbuild commented Jul 7, 2016

ebrondol commented Jul 7, 2016

Reducing duplicates in Phase2 tracking reconstruction (with some minor effects on phase0/1 tracking) #14994

Reducing duplicates in Phase2 tracking reconstruction (with some minor effects on phase0/1 tracking) #14994

Conversation

ebrondol commented Jun 27, 2016

cmsbuild commented Jun 27, 2016

slava77 Jun 27, 2016

Choose a reason for hiding this comment

ebrondol Jun 27, 2016

Choose a reason for hiding this comment

slava77 Jul 5, 2016

Choose a reason for hiding this comment

ebrondol Jul 5, 2016

Choose a reason for hiding this comment

VinInn Jul 7, 2016

Choose a reason for hiding this comment

slava77 commented Jun 27, 2016

cmsbuild commented Jun 27, 2016 • edited

cmsbuild commented Jun 27, 2016

slava77 commented Jun 27, 2016

cmsbuild commented Jun 27, 2016 • edited

cmsbuild commented Jun 27, 2016

cmsbuild commented Jun 27, 2016

slava77 commented Jun 28, 2016

makortel commented Jun 28, 2016

slava77 commented Jul 4, 2016

slava77 commented Jul 5, 2016

slava77 commented Jul 6, 2016

cmsbuild commented Jul 7, 2016

ebrondol commented Jul 7, 2016

VinInn commented Jul 7, 2016

cmsbuild commented Jul 7, 2016 • edited

cmsbuild commented Jul 7, 2016

cmsbuild commented Jul 7, 2016

slava77 commented Jul 7, 2016

cmsbuild commented Jul 7, 2016

ebrondol commented Jul 7, 2016

cmsbuild commented Jun 27, 2016 •

edited

cmsbuild commented Jun 27, 2016 •

edited

cmsbuild commented Jul 7, 2016 •

edited