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

Extended maximum allowed span for Phase2 Inner Tracker pixel clusters (81X) #16741

Conversation

ferencek
Copy link
Contributor

@ferencek ferencek commented Nov 23, 2016

This PR extends the maximum allowed span for Phase2 Inner Tracker pixel clusters from 127 to 255 to stay in sync with #16393.

Note that the DataFormat would actually allow the maximum to go above 255. However, given the fact that pixel clusters are limited to containing a maximum of 256 pixels, setting the maximum allowed span to 255 is sufficient.

Backport of #16742

@emiglior @atricomi @ebrondol

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ferencek (Dinko Ferencek) for CMSSW_8_1_X.

It involves the following packages:

DataFormats/Phase2ITPixelCluster

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@slava77
Copy link
Contributor

slava77 commented Nov 23, 2016

@ferencek
is it really needed in 81X?
90X is the new development release, including that for tracker TDR

@ferencek
Copy link
Contributor Author

@slava77, @emiglior can probably comment on whether this is needed in 81X. If needed, I can easily make a 90X PR.

@slava77
Copy link
Contributor

slava77 commented Nov 23, 2016

On 11/23/16 6:39 AM, Dinko Ferencek wrote:

@slava77 https://github.com/slava77, @emiglior
https://github.com/emiglior can probably comment on whether this is
needed in 81X. If needed, I can easily make a 90X PR.

90X PR is needed anyways.
There is no automatic forward porting.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16741 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbmA961XR2vAIeeZaEAXoVt-LLax9ks5rBFAQgaJpZM4K6o45.

@ferencek ferencek changed the title Extended maximum allowed span for Phase2 Inner Tracker pixel clusters Extended maximum allowed span for Phase2 Inner Tracker pixel clusters (81X) Nov 23, 2016
@slava77
Copy link
Contributor

slava77 commented Nov 24, 2016

if this is needed just in view of reclustering #16393, I think that this PR is not needed in 81X,
considering that there is no cluster splitting in phase2 tracking in the current setups.

@ferencek
Copy link
Contributor Author

@slava77, this is more related to keeping the two DataFormats in sync, regardless of any cluster splitting which might be added at some point in the future. Given the smaller pixel sizes in Phase 2 and the first layer closer to the beam pipe, cluster merging inside high-pt jets should be even more pronounced than in the current detector.

@cvuosalo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 25, 2016

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

+1

For #16741 b0cc619

Extending the maximum allowed span for Phase2 Inner Tracker pixel clusters from 127 to 255.

#16742 is the 90X version of this PR.

The code change is satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-11-25-1100 show no significant differences, as expected. Extended tests of workflows 1313.0_QCD_Pt_3000_3500_13 and 21230.0_QCD_Pt_3000_3500_14TeV+QCD_Pt_3000_3500_14TeV_TuneCUETP8M1_2023D4 with 1000 and 200 events, respectively, against baseline CMSSW_8_1_X_2016-11-13-0000 also show no significant differences.

@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, @smuzaffar

@davidlange6 davidlange6 merged commit 159eb68 into cms-sw:CMSSW_8_1_X Nov 27, 2016
@ferencek ferencek deleted the ExtendedPhase2ITPixelClusterSpan_from-CMSSW_8_1_0_pre16 branch November 29, 2016 21:00
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