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

narrower anode time window switched to True #17521

Merged
merged 2 commits into from Feb 20, 2017

Conversation

nvoytish
Copy link
Contributor

Use a narrower anode time window to build CSC recHits.
The flag CSCUseReducedWireTimeWindow switched to True.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @nvoytish for CMSSW_9_0_X.

It involves the following packages:

RecoLocalMuon/CSCRecHitD

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ptcox, @bellan, @abbiendi, @jhgoh 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

@slava77
Copy link
Contributor

slava77 commented Feb 15, 2017

@cmsbuild please test

@ptcox @nvoytish
please provide a link to slides or other documentation on the size of changes to expect from this PR.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 15, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17796/console Started: 2017/02/15 14:42

@ptcox
Copy link
Contributor

ptcox commented Feb 15, 2017

@slava77
Copy link
Contributor

slava77 commented Feb 15, 2017 via email

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@ptcox
Copy link
Contributor

ptcox commented Feb 15, 2017 via email

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Feb 17, 2017

Looking at the jenkins tests comparisons, I see no differences.
Do we expect any (over a few hundred MC and data events, not all with muons)?

The twiki is suggestive of some changes.

Looking in detail:
the twiki says Use narrower wire time window for reconstruction, proposed wire time window is from 6-10
The configuration currently in the code is

    CSCWireTimeWindowLow = cms.int32(0),
     CSCWireTimeWindowHigh = cms.int32(15),

It looks like this PR is incomplete.

@nvoytish @ptcox
please check and follow up

Thank you.

@ptcox
Copy link
Contributor

ptcox commented Feb 17, 2017 via email

@cmsbuild
Copy link
Contributor

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

@nvoytish
Copy link
Contributor Author

I changed the anode window to 6-10.

Nikolay

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 18, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17855/console Started: 2017/02/18 11:03

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Feb 19, 2017

+1

for #17521 33a3a20

  • change is as described: to use smaller CSC hit wire time window
  • jenkins tests pass and comparisons with baseline show small differences starting from CSC hits or segments and propagating to downstream quantities
  • higher statistics tests roughly confirm the expected behavior described in https://twiki.cern.ch/twiki/bin/viewauth/CMS/UFCSCLocalRecoStudy
    • muon gun samples show about o(0.1%) reduction in muon reconstruction efficiency
    • ZMM with PU35 test shows no visible reduction in muon efficiency but shows a reduction in the number of fakes
    • other samples with pileup show reduction in muon fakes as well
    • data samples show decrease in CSC segment counts similar to what's seen in MC with pileup
  • CSC segments become generally shorter with this PR, which is recovered by the changes in RU CSC segment builder update #17543

On the technical side, as seen in ttbar PU35 sample (10224), there is on average 10% decrease in CPU time of muon reconstruction modules (standalone reco times are down by 30%) for a total of about 1% decrease in the full reco time.

@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
Copy link
Contributor

+1

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