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

Remove inheritance in L1MuonPixelTrackFitter and L1MuonRegionProducer #16492

Merged
merged 2 commits into from Nov 10, 2016

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Nov 7, 2016

While refactoring PixelTrackProducer, it occurred to me that for regions and fitter TSGFromL1Muon uses only L1MuonRegionProducer and L1MuonPixelTrackFitter (with an explicit dynamic_cast in the code and without failure check). Furthermore, these two classes are used only by TSGFromL1Muon. Therefore, for current use cases, there is no need for L1MuonRegionProducer to inherit from TrackingRegionProducer, and L1MuonPixelTrackFitter from PixelFitter. This PR suggests to remove those inheritances to ease a bit further refactoring elsewhere.

Tested in CMSSW_8_1_0_pre16, no changes expected in results.

@rovere @VinInn

The only customer of L1MuonPixelTrackFitter is TSGFromL1Muon, which
dynamic_casts the fitter to L1MuonPixelTrackFitter. The current code
does not work unless the the fitter is of type L1MuonPixelTrackFitter,
this commit makes this fact more explicit.
The only customer of L1MuonRegionProducer is TSGFromL1Muon, which
dynamic_casts the region producer to L1MuonRegionProducer. The current
code does not work unless the the region producer is of type
L1MuonRegionProducer, this commit makes this fact more explicit.
@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2016

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_8_1_X.

It involves the following packages:

RecoMuon/TrackerSeedGenerator

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@bellan, @abbiendi, @jhgoh, @calderona, @HuguesBrun, @battibass, @trocino, @bachtis, @rociovilar this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@VinInn
Copy link
Contributor

VinInn commented Nov 7, 2016

@cmsbuild ,please test

@cvuosalo
Copy link
Contributor

cvuosalo commented Nov 8, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

@cvuosalo
Copy link
Contributor

+1

For #16492 4e91206

Code clean-up of unneeded inheritance in L1MuonPixelTrackFitter and L1MuonRegionProducer. There should be no change in monitored quantities.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-11-08-1100 show no significant differences, as expected.

@slava77
Copy link
Contributor

slava77 commented Nov 10, 2016

did the bot fall asleep?

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

+1

@cmsbuild cmsbuild merged commit 6a1f7f6 into cms-sw:CMSSW_8_1_X Nov 10, 2016
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

6 participants