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

bugfixes and restoring backwards compatibility of Combinatorial DT reco #3724

Merged
merged 1 commit into from May 13, 2014

Conversation

ptraczyk
Copy link
Contributor

@ptraczyk ptraczyk commented May 7, 2014

This is an update of the "physics changes" that were taken out of #3450.
The performance of HLT muons should return to the state from pre4, as HLT muons use the (old) combinatorial pattern recognition DT algorithm.
The same should happen to cosmic muons as they also use the old algorithm, though this is changing right now (#3722).

Reco muons change little from pre5/pre6 and the changes should in general be improvements (slightly better segments chi2, better segment angular resolution etc)

@cmsbuild
Copy link
Contributor

cmsbuild commented May 7, 2014

A new Pull Request was created by @ptraczyk (Piotr Traczyk) for CMSSW_7_1_X.

bugfixes and restoring backwards compatibility of Combinatorial DT reco

It involves the following packages:

RecoLocalMuon/DTSegment

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@slava77
Copy link
Contributor

slava77 commented May 7, 2014

Hi Piotr,

In part for documentation purpose, in part to show some pre-validation, could you please post some plots supporting your expectations from changes in the code proposed here.

  • for segments used for regular RECO muons (using either a mu gun or other signal-like muon sample)
  • for segments used in HLT
  • for segments used in cosmics (using a comsic sample)

Thank you very much.

@ptraczyk
Copy link
Contributor Author

ptraczyk commented May 8, 2014

Hi Slava,
ok, here goes. All plots for the ZMM RelVals, showing several parameters of STA muons. CSCs are removed from muon reconstruction.
I start with CMB segments used in HLT and Cosmics. First a comparison between pre4 and pre5 (the unintended changes that were introduces) and then pre4 and current PR (labeled as hltf) - differences are gone.

image

image

@ptraczyk
Copy link
Contributor Author

ptraczyk commented May 8, 2014

And here are the checks on the MT segments used in RECO.

Comparing this PR with pre5
image
and for comparison also vs pre4
image

and numbers of hits/segments per track
image

image

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2014

@abbiendi
Copy link
Contributor

This is a significant bug-fix, and cleans up things to move on consistently. It is fully supported by the Muon POG and the DT DPG, please take it.
Giovanni

@slava77
Copy link
Contributor

slava77 commented May 13, 2014

+1

for #3724 764ddc3
tested in CMSSW_7_1_X_2014-05-12-0200 (test area sign371)

changes are in line with expectations

  • more segments in general (more so in cosmics sequence, also in HLT and regular reco)
  • L2 and L3 tracks have more DT hits (and a few more tracks are reconstructed, ~+0.2% in 10 GeV gun)
  • regular muons are more or less unchanged

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

davidlange6 added a commit that referenced this pull request May 13, 2014
bugfixes and restoring backwards compatibility of Combinatorial DT reco
@davidlange6 davidlange6 merged commit c7bbd63 into cms-sw:CMSSW_7_1_X May 13, 2014
@slava77 slava77 mentioned this pull request Jan 8, 2015
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