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

Back to use PXB layer combination 134 and 124 for triplets (phase2 tracking) #17228

Merged
merged 1 commit into from
Feb 16, 2017

Conversation

ebrondol
Copy link
Contributor

The validation between pre1 and pre2 shows some inefficiency in the barrel region for single mu w/o PU0. This has been presented here. In order to recover it, two of the Pixel barrel layer combination have been re-introduced. Here are the results for eff/fake on a ttbar saple w/o PU: baseline (blue), +PR (red)
screenshot from 2017-01-20 13 53 50
These two layer combination were considered redundant and eliminated in order to speed up the tracking process. A deep validation showed that the effect of removing them was not negligible due to a pixel local reconstruction inefficiency which is under investigation.

@atricomi @suchandradutta @delaere if you have any more information on the local reconstruction, please comment this PR.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ebrondol 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, @makortel, @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

@ebrondol
Copy link
Contributor Author

ebrondol commented Jan 20, 2017

@slava77 I did not test the timing effect, I suppose this PR will not make much difference.
I also did not have a chance to produce the results at high PU.
It would be nice if this PR menage to be in pre3 so we can validate again soon.

@makortel
Copy link
Contributor

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 20, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17344/console Started: 2017/01/20 17:45

@VinInn
Copy link
Contributor

VinInn commented Jan 20, 2017

@cmsbuild , please test

@VinInn
Copy link
Contributor

VinInn commented Jan 20, 2017

In any case at the moment it is mandatory to recover the efficiency in the barrel

@slava77
Copy link
Contributor

slava77 commented Jan 20, 2017

if the timing is roughly under control (should be, since most badness is in the endcaps), it'd be good to go.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@@ -35,7 +35,7 @@
trackingPhase2PU140.toModify(highPtTripletStepSeedLayers,
# combination with gap removed as only source of fakes in current geometry (kept for doc)
layerList = ['BPix1+BPix2+BPix3', 'BPix2+BPix3+BPix4',
# 'BPix1+BPix3+BPix4', 'BPix1+BPix2+BPix4',
'BPix1+BPix3+BPix4', 'BPix1+BPix2+BPix4',
Copy link
Contributor

Choose a reason for hiding this comment

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

is either of these two more useful than the other?
Given the large increase in fake rate, maybe some optimization can be done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We tried to include just one but this was not enough to recover all the efficiency.

@slava77
Copy link
Contributor

slava77 commented Jan 24, 2017

in PU200 setup:

there are ~2% more general tracks, mostly coming from the highpt triplet iteration
all_sign830vsorig_ttbar14tev2023d4timingpu200localwf23034p0c_recotracks_generaltracks__reco_obj_algo

most of them are short with 4-5 hits
all_sign830vsorig_ttbar14tev2023d4timingpu200localwf23034p0c_recotracks_generaltracks__reco_obj_found

the number of gsf tracks doubles in the barrel
all_sign830vsorig_ttbar14tev2023d4timingpu200localwf23034p0c_recogsftracks_electrongsftracks__reco_obj_eta

General track efficiency is slightly higher (apparently, less of an improvement than it is seen in the muon reco)
wf23034pu200_general_alltp_eff_eta

Fake rate goes up in the barrel by about a factor of 2
wf23034pu200_generalpt09_fr_eta

wf23034pu200_gsf_eff_eta

The underlying seed rate is a bit more flat now
wf23034pu200_hiptt_seed_eta

btagging may be worse, but the 100 events in the test are not enough for a more conclusive statement
wf23034pu200_cmvav2_roc

Some numerology to go with the plots, showing changes in the yields

  • particleFlowEGamma: +7.5% (2799, mostly at lower energy <10 GeV
  • electronGsfTracks ~+2.2% (162K; ~doubles in the barrel
  • pf ch +1.1% (620K; with a jump around 0.9 GeV (highPt iter threshold?
  • pf ele :+24% (294
  • pf mu: +0.3%
  • pf pho: -0.5%
  • generalTracks +2% (773K, mostly pt>0.9
  • offline PVs +5%, mostly with larger uncertainties
  • btaggers have some noticeable changes
  • ecalDrivenGsfElectrons ~+1% (about 15% more in the barrel)
  • gedGsfElectrons: +22% (759

In ten mu wf 22811
the increase of track efficiency is clear, in agreement with the plot in the PR description showing ~1% increase in the barrel
wf22811_generalpt09_eta

On a technical side:

  • CPU is up by 5% driven on the iter tracking side by ~30% CPU increase in high pt triplets and outside iter tracking by ~15% increase in time used by the timing-related PV reco and 10% in regular PV reco

IMO, this is a rather questionable change: ~1% efficiency increase visible mostly just in muons at a cost of 15% of fake rate. For muons, shouldn't the efficiency be more effectively recovered with outside-in iteration?

@VinInn
Copy link
Contributor

VinInn commented Jan 24, 2017

IMO, this is a rather questionable change: ~1% efficiency increase visible mostly just in muons at a cost of 15% of fake rate. For muons, shouldn't the efficiency be more effectively recovered with outside-in iteration?

cannot disagree in general terms, that was the rationale that move me to remove those combinations: outIn is not yet commissioned in phaseII, pixelless iterations as well.
On the other hand it is clear that at some point we need to address failure scenarios as well.
I think this issue need to be addressed in the context of the TDR: personally I think that for physics most probably is better w/o this PR, still the single muon efficiency plot for the TDR will raise questions...

@slava77
Copy link
Contributor

slava77 commented Jan 24, 2017 via email

@VinInn
Copy link
Contributor

VinInn commented Jan 24, 2017 via email

@slava77
Copy link
Contributor

slava77 commented Jan 25, 2017

@VinInn @ebrondol
please clarify if we should close this PR at this point

@ebrondol
Copy link
Contributor Author

ebrondol commented Jan 25, 2017

In my personal opinion, I think that the fake rate is something that we need to fix. And we are working on that pretty hardly and once we have fixed that, even if this PR increase the fake (maybe it won't at that point anymore) it will not be a huge problem as it is now.
I would really prefer not to go in production with a muon efficiency lower than 99.9% in the barrel. It is really difficult to explain, especially if we know that we can provide a fix for that!
Please, give me some days to cure the fake rate effect and then we can discuss about this PR again.

@slava77
Copy link
Contributor

slava77 commented Jan 27, 2017

@ebrondol
regarding
prefer not to go in production with a muon efficiency lower than 99.9% in the barrel
Are you saying that the 820 will not be used anymore by tracking community due to ~1% inefficiency?

@ebrondol
Copy link
Contributor Author

ebrondol commented Feb 8, 2017

@slava77
As I presented in the UPSG meeting:
https://indico.cern.ch/event/611574/
the effect of the fake as been found. This is mainly due to pixel localReco filter. If this is going to be solved, this PR can be re-valuated, but it is not up to my power.
I would try to introduce the muon outIn iter.

@slava77
Copy link
Contributor

slava77 commented Feb 16, 2017

general track fake rate, to complement pt09 plot posted earlier
https://cloud.githubusercontent.com/assets/4676718/22228975/9c99c286-e189-11e6-8695-4cf4d3a7f555.png

wf23034_general_fake_eta

@slava77
Copy link
Contributor

slava77 commented Feb 16, 2017

+1
(now that #17504 is merged)

for #17228 1c68f2e

  • updates are as described
  • jenkins tests show differences only in 2023 workflows (I happen to still have a copy of tests by jenkins in CMSSW_9_0_X_2017-01-20-1100 )
  • local tests show roughly expected behavior:

@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 davidlange6 merged commit d748a88 into cms-sw:CMSSW_9_0_X Feb 16, 2017
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.

6 participants