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

Do not use CSC Bad Chambers list during muon reconstruction #11248

Merged
merged 7 commits into from Oct 2, 2015

Conversation

jhgoh
Copy link
Contributor

@jhgoh jhgoh commented Sep 14, 2015

CSC Bad chamber list was used in the muon reconstruction by default but this information but using this information is not recommended by the CSC DPG.
Excluding bad chamber may cause unexpected inefficiency if the information is not up to date.

The feature to exclude 'bad' CSCs are turned off by changing the default parameter in cff file.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jhgoh (Junghwan John Goh) for CMSSW_7_6_X.

Do not use CSC Bad Chambers list during muon reconstruction

It involves the following packages:

TrackingTools/TrackAssociator

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
@battibass, @makortel, @abbiendi, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @istaslis, @gpetruc, @cerati, @trocino, @dgulhan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@@ -33,7 +33,7 @@
etaBinSize = cms.double(0.125),
nEta = cms.int32(48),
nPhi = cms.int32(48),
includeBadChambers = cms.bool(False)
includeBadChambers = cms.bool(True)
Copy link
Contributor

Choose a reason for hiding this comment

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

a change made globally in this file will introduce problems in run1 simulation.
Run-2 specific changes should be added in postLS1 customizations

Copy link
Contributor

Choose a reason for hiding this comment

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

... maybe we actually add this to Configuration/StandardSequences/python/Reconstruction_Data_cff.py ?
I think it's more appropriate than even doing it just for run2

@jhgoh
Copy link
Contributor Author

jhgoh commented Sep 15, 2015

@slava77 includeBadChamber parameter is changed in the PostLS1 customization.

@cmsbuild
Copy link
Contributor

Pull request #11248 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Sep 15, 2015

@cmsbuild please test
(the one sent from email client didn't make it )

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@civanch
Copy link
Contributor

civanch commented Sep 16, 2015

+1
however, the name of the parameter is misleading - these chambers are not "bad", I would change this name

@cmsbuild
Copy link
Contributor

Pull request #11248 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2015

  • Looking at jenkins outputs: there are no changes as expected
    • run1 setup is not affected by construction
    • run2 MC has no bad chambers, so nothing to skip before the change
  • tested locally in CMSSW_7_6_0_pre5 on 200 events in 251721, 256729, 254790: there is a change in an expected place the number of crossed chambers by the muon increases (1 muon that turned out to be in DoubleEG251721). There is no effect on other monitored quantities
    wfdoubleeg251721_nchambers_norpc

@slava77
Copy link
Contributor

slava77 commented Sep 25, 2015

"+1" (even though it's not a proper cms-bot signature)

@davidlange6
Copy link
Contributor

+1

@civanch
Copy link
Contributor

civanch commented Oct 2, 2015

+1

cmsbuild added a commit that referenced this pull request Oct 2, 2015
Do not use CSC Bad Chambers list during muon reconstruction
@cmsbuild cmsbuild merged commit 3518337 into cms-sw:CMSSW_7_6_X Oct 2, 2015
@jhgoh jhgoh deleted the BadChamberFix branch October 11, 2015 15:58
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