Simplify class stucture of hit generators #10016
Conversation
…tor and CosmicHitTripletGeneratorFromLayerTriplet These classes do not benefit from the base classes (HitTripletGenerator, OrderedHitsGenerator). The classes are not registered to any plugin factory, and hence can be constructed only explicitly. The only user (SeedGeneratorForCosmics) use the type CosmicHitTripletGenerator explicitly, so there is no need for dynamic polymorphism from there either.
…ratorForPhotonConversion CombinedHitQuadrupletGeneratorForPhotonConversion does not really benefit from the base classes, and the sole user (PhotonConversionTrajectorySeedProducerFromQuadrupletsAlgo) uses the concrete type explicitly, so no need for dynamic polymorphism either. In fact, now we can remove the non-implemented methods that are pure virtual in the base class.
…mLayerPairForPhotonConversion HitQuadrupletGeneratorFromLayerPairForPhotonConversion does not really benefit from the base classes, and the only user (CombinedHitQuadrupletGeneratorForPhotonConversion) uses the type explicitly, so there is no need for dynamic polymorphism either. In fact, now we can remove the non-implemented methods that are pure virtual in the base class.
CosmicHitPairGenerator does not benefit from the base class, and the users (SeedGeneratorForCRack and SeedGeneratorForCosmics) use the concrete types, so there is no need for dynamic polymorphism either. In fact, now we can remove some unneeded methods that are pure virtual in the base class.
…mLayerPair CosmicHitPairGeneratorFromLayerPair does not benefit from base classes, and the user (CosmicHitPairGenerator) uses the type explicitly, so there is no need for dynamic polymorphism either. In fact, we can now remove some unnecessary methods that were pure virtual in the base class.
…rAndLayers MultiHitGeneratorFromPairAndLayers does not really benefit from the base class, and the user (CombinedMultiHitGenerator) use it as the base class type for dynamic polymorphism. Now we can also remove some unneeded methods that are pure virtual in the base class.
…mPairAndLayers HitTripletGeneratorFromPairAndLayers does not really benefit from the base class, and the user (CombinedHitTripletGenerator) use it as the base class type for dynamic polymorphism. Now we can also remove some unneeded methods that are pure virtual in the base class.
…Pair HitPairGeneratorFromLayerPair does not really benefit from the base class. The users in practice use exactly this class (and not other classes inheriting from HitPairGenerator), so they can safely be modified to use the concrete type eliminating the need for dynamic polymorphism. Also the HitPairGenerator::setSeedingLayers() can now be removed as its asserting implmentation in CombinedHitPairGenerator.
…on as in other HitPair generators
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_7_6_X. Simplify class stucture of hit generators It involves the following packages: RecoPixelVertexing/PixelLowPtUtilities @cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks. |
|
Lovely! |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
I have no idea how these changes could affect (the performance of) |
|
+1 |
Simplify class stucture of hit generators
|
On 7/6/15 1:05 AM, Matti Kortelainen wrote:
this is quite likely from the database latency. Carl, how did the memory look like?
|
|
@Slava: Summary for 70 events PR 10016 Product size or, B new, B delta, B delta, % deltaJ, % branch |
|
On 7/7/15 10:40 AM, Carl Vuosalo wrote:
Thanks.
|
Current inheritance chain of the hit generators is somewhat messy, and in some parts does not serve the current usage of the classes. This PR removes unnecessary inheritances and does some further cleanup allowed by the removal.
Essentially, the inheritance from HitPairGenerator is removed from
the inheritance from HitTripletGenerator from
and the inheritance from MultiHitGenerator from
(Most of the developments were actually done already more than a year ago, but for some reason I didn't push them for integration and they got buried under other stuff)
Tested in 750pre5, no changes expected in results.
@rovere @VinInn