Change of the parameters of SiPixel local reco to match the specs of the phase2 read-out chip. #16943

emiglior · 2016-12-09T10:50:28Z

This PR is meant to provide a realistic description of the specs of the ROC for the phase2 Inner Tracker pixels, more precisely to so called "T3 tracker" featuring the v4.0.2.1 of the Inner Tracker:

1 ADC = 600 e
Full range of A/D converter = 9600e (e.g. 4bit A/D converter)
No noisy pixel (e.g. no hits made only of noisy pixel cells)

Thresholds of the pixel digitizer have been changed accordingly.
Errors on local position reconstructed by PixelCPEGeneric algorithm were recalibrated.
Details on the new settings can be found in:

DPG: @boudoul @atricomi @delaere @suchandradutta
TRK POG: @ebrondol @makortel @VinInn @rovere

…phase2 ROC

cmsbuild · 2016-12-09T10:50:46Z

A new Pull Request was created by @emiglior (Ernesto Migliore) for CMSSW_9_0_X.

It involves the following packages:

RecoLocalTracker/SiPixelClusterizer
RecoLocalTracker/SiPixelRecHits
SimTracker/SiPhase2Digitizer

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

cms-bot commands are listed here #13028

VinInn · 2016-12-09T11:19:40Z

@cmsbuild , please test

cmsbuild · 2016-12-09T11:19:54Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16894/console Started: 2016/12/09 12:20

VinInn · 2016-12-09T11:21:26Z

FYI @ebrondol @boudoul @veszpv (DPG coords, please subscribe to RecoLocalTracker)

cmsbuild · 2016-12-09T12:40:28Z

+1
Tested at: 640c61b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16943/16894/summary.html

cmsbuild · 2016-12-09T12:40:30Z

Comparison job queued.

cmsbuild · 2016-12-09T13:16:50Z

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

civanch · 2016-12-09T20:45:50Z

+1

slava77 · 2016-12-09T21:02:17Z

RecoLocalTracker/SiPixelRecHits/src/PixelCPEGeneric.cc

-    yerr_endcap_def_=0.00357;
+  } else { // isUpgrade=true    
+    xerr_barrel_ln_= {0.00025, 0.00030, 0.00035, 0.00035};
+    xerr_barrel_ln_def_=0.0035;


Is the default of 35um correct? Looks much larger than 3.5um in the last binned value.

emiglior · 2016-12-12T22:51:26Z

Thanks for reporting on this detailed list of tests.

I would like to add few comments/questions:

I think that several changes (increase of chi2, decrease of the errors on the PV, increase of the pull dxy vs eta) just reflect that the uncertainties on the RecHit local position are smaller than before. BTW: it would be nice to see also the prob(chi2) which was the main objective function in the calibration procedure.
About the inefficiency in the selectedOfflinePrimaryVertices, they are concentrated in few bins. Not being an expert of the tracking DQM, naively I am wondering if they these bins correspond to categories of vertices which share some common features.
HIP_mass_perTrack: not sure about what is entering in this distribution, but keep in mind that in the PR we have also changed the parameters of the SiPixelDigitizer. As a consequence we expect that deterioration of the dEdx measurement (especially in eta regions where the pixel measurements dominate).
PackedCandidate: I can't comment on it as I don't know what these distributions represent (apologies...). Where is it possible to find some documentation?

slava77 · 2016-12-12T22:55:25Z

On 12/12/16 2:51 PM, Ernesto Migliore wrote: Thanks for reporting on this detailed list of tests. I would like to add few comments/questions: * I think that several changes (increase of chi2, decrease of the errors on the PV, increase of the pull dxy vs eta) just reflect that the uncertainties on the RecHit local position are smaller than before. BTW: it would be nice to see also the prob(chi2) which was the main objective function in the calibration procedure.

Pulls growing above one is a good indication that uncertainties are underestimated or that positions have intrinsic biases that were accidentally "demoted" by the old larger uncertainties. What is the more likely explanation?

…

* About the inefficiency in the selectedOfflinePrimaryVertices, they are concentrated in few bins. Not being an expert of the tracking DQM, naively I am wondering if they these bins correspond to categories of vertices which share some common features. * HIP_mass_perTrack: not sure about what is entering in this distribution, but keep in mind that in the PR we have also changed the parameters of the SiPixelDigitizer. As a consequence we expect that deterioration of the dEdx measurement (especially in eta regions where the pixel measurements dominate). * PackedCandidate: I can't comment on it as I don't know what these distributions represent (apologies...). Where is it possible to find some documentation? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16943 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbtXGWWRiZw2MXQCnadQ77-KJzrjvks5rHc_vgaJpZM4LIzz->.

emiglior · 2016-12-12T23:09:27Z

Pulls growing above one is a good indication that uncertainties are underestimated or that positions have intrinsic biases that were accidentally "demoted" by the old larger uncertainties. What is the more likely explanation?

The distributions of RecHit-SimHit residuals did not show significant biases. This seems to favor the fact that uncertainties are underestimated but it is not trivial to "inflate" them as the average prob(chi2) is already > 0.5.

slava77 · 2016-12-13T01:09:44Z

PU200 workflow 23034 100 events (baseline is CMSSW_9_0_X_2016-12-10-1100)

Some technical numbers first:

siPixelClusters times is up by x20 from 0.17 to 3 s/evt .. this is still small wrt total reco time
siPixelRecHits up by x4 from 0.16 to 0.73
total reco time is down by about 10% from 820 s to 740 s.

downstream times have generally decreased in tracking-related modules (2nd column is per-job cost reduction, where the job time includes DQM&Validation)

delta/moduleMean  delta/jobMean   baseline     PR
   -0.099543      -0.12%     18201.20 ms/ev ->     16475.30 ms/ev pixelTracks
   -0.362499      -0.15%      6633.59 ms/ev ->      4597.89 ms/ev pixelVertices
   -1.066531      -0.23%      4580.47 ms/ev ->      1394.32 ms/ev detachedQuadStepTrackCandidates
   -0.705560      -3.57%     95980.20 ms/ev ->     45920.50 ms/ev lowPtQuadStepTrackCandidates
   -0.471853      -0.37%     13747.60 ms/ev ->      8499.03 ms/ev particleFlowDisplacedVertex
   -0.357285      -0.05%      2466.15 ms/ev ->      1718.58 ms/ev lowPtQuadStepTracks
   -0.033253      -0.17%     74000.20 ms/ev ->     71579.70 ms/ev lowPtTripletStepTrackCandidates
   -0.012042      -0.06%     68537.30 ms/ev ->     67716.90 ms/ev highPtTripletStepTrackCandidates
   -0.181503      -0.51%     42917.90 ms/ev ->     35776.30 ms/ev initialStepTrackCandidates
   -0.123902      -0.05%      5935.19 ms/ev ->      5242.71 ms/ev earlyGeneralTracks

   -0.310640      -0.06%      3222.91 ms/ev ->      2356.34 ms/ev gsfGeneralInOutOutInConversionTrackMerger
   +0.179032      +0.17%     12442.90 ms/ev ->     14889.60 ms/ev electronGsfTracks
   -0.145325      -0.14%     13988.70 ms/ev ->     12093.50 ms/ev muons1stStep
   -0.072029      -0.16%     33157.60 ms/ev ->     30852.30 ms/ev pfTrackElec

(red is this PR; baseline is CMSSW_9_0_X_2016-12-10-1100 in black)

Something on the low level:
increase of size=1 pixel clusters is about 30%, while the larger size population goes down

and the at the higher level:

the side with very large uncertainties (or too correct tracks) on prob ~ 1 is flattened out substantially
however, the tails in chi2 increase

charge misid is up substantially

duplicate rates are up

efficiency is about the same

fakes are down

.. related to dxy
oddly enough, the sigma(dxy) is down

while the pull is up

The effect is smaller for "general_" track plots (without a pt cut).

Clearly, dxy is affected the most by the pixel position and CPE.
So, the fact that most significant differences in track parameters are in dxy are expected.
Looking at other pulls, they are not that close to 1 either. So, having one more quantity shift away above 1 is probably not that bad.

Having some plots with residuals in the standard validation/DQM would be much nicer.

cov(phi,dxy) is packed in the range of exp(-17)~4E-8 to exp(-4)~0.018.
I scanned the file manually and I see many more tracks with cov(phi,dxy) below 4E-8.
This explains the plot of packed candidates.

There will need to be some work done downstream in tracking to handle the duplicates better and maybe improve on the charge misid.

Overall, it seems to me that the PR is good to go.

some comments from @VinInn @rovere @ebrondol would be nice, before we sign off.

davidlange6 · 2016-12-13T13:08:07Z

@VinInn @rovere @ebrondol @slava77 - should we move forward here?

ebrondol · 2016-12-13T13:15:56Z

I am sorry, I did not have the time to produce comparison for this PR yet.
I can do it, but I need some hours.

rovere · 2016-12-13T13:35:43Z

@davidlange6
This is a PR related to localReco and DPG and I think it should be their call to say if this PR represents an improvement in local-reconstruction or not. We cannot validate local-reco changes via tracking-only quantities: we said it clear and loud years ago.
From the tracking point of view the effects have been clearly identified and explained by Slava and Carl.
If localReco and especially errors are modified, everything down the chain has to be tuned/adapted. I personally do not see huge show stoppers here. If times allow, it would be nice to see more tracking-oriented details from Erica, if she's ok to produce them.

davidlange6 · 2016-12-13T13:40:44Z

so we are -12 hours to the deadline for the planned prod release.. if we envision tracking adjustments by accepting this PR, I guess we don't take it (thus @slava77s request for input from tracking).

sextonkennedy · 2016-12-13T13:41:23Z

Hi All, There was a push from PPD at the XEB to get pre2 out ASAP. I understand that this is what we are waiting for, and it seems that Marco is saying we should move ahead... so please do.

davidlange6 · 2016-12-13T13:43:19Z

move ahead "with" or "without" this pr?

slava77 · 2016-12-13T13:47:08Z

@rovere
My request for comments to TRK POG was mainly driven to get your assessment of the behavior of the CPE errors.
It's true that this is a DPG level PR, but the errors are derived using tracks (single muons, 10 GeV, IIRC) and you may know or see if there are any potential issues based on the observations with reconstructed tracks.

So far I hear that there are no issues.

slava77 · 2016-12-13T13:49:28Z

@delaere @Tricomi @emiglior
this PR is essential for the TDR sample production (for 900pre2 or 820), right?
If not, it will have to wait for the second round.

rovere · 2016-12-13T13:54:46Z

@slava77 judging from the plots reported in this thread I see no particular issues. Yes, the pulls get wider, but that was also spotted here and most likely they were artificially smaller before.
As for further tuning also based on this PR I do not consider tracking development for 90 and PhaseII closed with pre2.

sextonkennedy · 2016-12-13T13:56:33Z

My understanding was that it was essential for the TDR, and that the DPG organization (for which this is the focus) wants this in. @davidlange6 the answer is move "with". If it requires a retuning of the high level reco so be it. There is a planned after the holiday second iteration, and it seems this gets us one step closer in that cycle.

boudoul · 2016-12-13T13:57:10Z

Dear @slava77 , @sextonkennedy and @davidlange6 , give me ~15' to come back to you to give the final word from TrackerDPG

davidlange6 · 2016-12-13T13:58:55Z

typically consistency across the reco is preferred for productions - but anyway, sounds like all is well in this case given the recent comments.

…

On Dec 13, 2016, at 3:56 PM, sextonkennedy ***@***.***> wrote: My understanding was that it was essential for the TDR, and that the DPG organization (for which this is the focus) wants this in. @davidlange6 the answer is move "with". If it requires a retuning of the high level reco so be it. There is a planned after the holiday second iteration, and it seems this gets us one step closer in that cycle. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

sextonkennedy · 2016-12-13T13:59:43Z

Sure @boudoul, you get the final word... and we can wait 15' :-)

slava77 · 2016-12-13T14:10:21Z

if tracking would need massive updates after updates at the local level, the change can not be added in a release meant for production including studies with tracks.
Else we'd run a much cheaper campaign with just tracker local reco.

As seen from our plots it seems like track reconstruction is still adequately functional.

boudoul · 2016-12-13T14:35:22Z

Dear all , finally coming back to you ,
We (trackerDPG) consider this development to be indeed needed for TDR and the next MC prod.
Thanks for taking care of all of this (and all useful comments received during the review of the PR)

In short +1 from Tracker DPG

slava77 · 2016-12-13T14:37:38Z

@ebrondol please post the MTV plots when available.

davidlange6 · 2016-12-13T14:43:45Z

as to work in parallel with @ebrondol i'll merge this

rovere · 2016-12-13T22:32:33Z

I attach here a very quick, 10 events comparison in 90X with and w/o the PR. As noted already, there are no huge issues with the PR as is which, in the meantime, got merged for good.
Erica had to leave for a conference.
Link to plots

cvuosalo · 2016-12-15T20:12:36Z

+1

For #16943 6e7a311

For the Phase 2 Inner Tracker, providing more accurate SiPixel settings.

The code changes are satisfactory. Jenkins tests against baseline CMSSW_9_0_X_2016-12-08-2300 show numerous small differences in Phase 2 workflows. Extensive additional tests were performed and assessed by experts, as discussed above. The conclusion is that the changes caused by this PR are acceptable and expected.

cmsbuild · 2016-12-15T20:12:50Z

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. @slava77, @davidlange6, @smuzaffar

Change of the parameters of SiPixel local reco to match the specs of …

640c61b

…phase2 ROC

cmsbuild added this to the Next CMSSW_9_0_X milestone Dec 9, 2016

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending simulation-pending tests-pending labels Dec 9, 2016

cmsbuild added tests-started and removed tests-pending labels Dec 9, 2016

cmsbuild added tests-approved and removed tests-started labels Dec 9, 2016

cmsbuild added comparison-available and removed comparison-pending labels Dec 9, 2016

cmsbuild added simulation-approved and removed simulation-pending labels Dec 9, 2016

slava77 reviewed Dec 9, 2016

View reviewed changes

Fix typo in one default value in PixelCPE Generic

6e7a311

cmsbuild added comparison-pending and removed comparison-available simulation-approved tests-approved labels Dec 10, 2016

davidlange6 merged commit 3b7afb7 into cms-sw:CMSSW_9_0_X Dec 13, 2016

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Dec 15, 2016

ebrondol mentioned this pull request Jan 4, 2017

Second tuning of Pixel cluster shape cut for Phase 2 #17110

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change of the parameters of SiPixel local reco to match the specs of the phase2 read-out chip. #16943

Change of the parameters of SiPixel local reco to match the specs of the phase2 read-out chip. #16943

emiglior commented Dec 9, 2016

cmsbuild commented Dec 9, 2016

VinInn commented Dec 9, 2016

cmsbuild commented Dec 9, 2016 •

edited

Loading

VinInn commented Dec 9, 2016 •

edited

Loading

cmsbuild commented Dec 9, 2016

cmsbuild commented Dec 9, 2016

cmsbuild commented Dec 9, 2016

civanch commented Dec 9, 2016

slava77 Dec 9, 2016

emiglior commented Dec 12, 2016

slava77 commented Dec 12, 2016 via email

emiglior commented Dec 12, 2016

slava77 commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

ebrondol commented Dec 13, 2016

rovere commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

sextonkennedy commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

slava77 commented Dec 13, 2016

slava77 commented Dec 13, 2016 •

edited

Loading

rovere commented Dec 13, 2016

sextonkennedy commented Dec 13, 2016

boudoul commented Dec 13, 2016

davidlange6 commented Dec 13, 2016 via email

sextonkennedy commented Dec 13, 2016

slava77 commented Dec 13, 2016

boudoul commented Dec 13, 2016

slava77 commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

rovere commented Dec 13, 2016

cvuosalo commented Dec 15, 2016

cmsbuild commented Dec 15, 2016

Change of the parameters of SiPixel local reco to match the specs of the phase2 read-out chip. #16943

Change of the parameters of SiPixel local reco to match the specs of the phase2 read-out chip. #16943

Conversation

emiglior commented Dec 9, 2016

cmsbuild commented Dec 9, 2016

VinInn commented Dec 9, 2016

cmsbuild commented Dec 9, 2016 • edited Loading

VinInn commented Dec 9, 2016 • edited Loading

cmsbuild commented Dec 9, 2016

cmsbuild commented Dec 9, 2016

cmsbuild commented Dec 9, 2016

civanch commented Dec 9, 2016

slava77 Dec 9, 2016

Choose a reason for hiding this comment

emiglior commented Dec 12, 2016

slava77 commented Dec 12, 2016 via email

emiglior commented Dec 12, 2016

slava77 commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

ebrondol commented Dec 13, 2016

rovere commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

sextonkennedy commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

slava77 commented Dec 13, 2016

slava77 commented Dec 13, 2016 • edited Loading

rovere commented Dec 13, 2016

sextonkennedy commented Dec 13, 2016

boudoul commented Dec 13, 2016

davidlange6 commented Dec 13, 2016 via email

sextonkennedy commented Dec 13, 2016

slava77 commented Dec 13, 2016

boudoul commented Dec 13, 2016

slava77 commented Dec 13, 2016

davidlange6 commented Dec 13, 2016

rovere commented Dec 13, 2016

cvuosalo commented Dec 15, 2016

cmsbuild commented Dec 15, 2016

cmsbuild commented Dec 9, 2016 •

edited

Loading

VinInn commented Dec 9, 2016 •

edited

Loading

slava77 commented Dec 13, 2016 •

edited

Loading