Harmonize the use of reco::TrackBase::TrackAlgorithm enumeration #8315
Conversation
…th explicit list of enumerators The order of the enumerators do not, in general, contain (the naively expected) information. It is better to spell out explicitly all the enumerators of interest. This issue will become relevant when we add enumerators for the Phase1-specific iterations.
The order can, in general, be different from the order of the enumerators (and will when the Phase1 enumerators get added).
|
We considered using the C++11 strongly-typed enum for the TrackAlgorithm, but decided to postpone that until there is no need to protect it with #ifdef in the header. |
| @@ -42,7 +42,7 @@ | |||
| chi2_tight_max = cms.double(3.0), | |||
| nhits_tight_min = cms.double(11), | |||
| ptErr_tight_max = cms.double(0.1), | |||
| maxTrackAlgo = cms.int32(8) | |||
| trackAlgos = cms.vstring("undefAlgorithm", "ctf", "rs", "cosmics", "initialStep", "lowPtTripletStep", "pixelPairStep", "detachedTripletStep"), | |||
There was a problem hiding this comment.
I'm not perfectly happy with ending up to two parameters with similar names (trackAlgos and track_algos) and different semantics, but I think this issue kind-of existed already before.
|
A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_7_5_X. Harmonize the use of reco::TrackBase::TrackAlgorithm enumeration It involves the following packages: DQM/PhysicsHWW @nclopezo, @cvuosalo, @monttj, @cmsbuild, @deguio, @slava77, @vadler, @danduggan can you please review it and eventually sign? Thanks. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @nclopezo, @ktf, @smuzaffar |
|
+1 |
Harmonize the use of reco::TrackBase::TrackAlgorithm enumeration
This PR is a preparatory step towards adding enumerators for Phase1-specific iterations to reco::TrackBase::TrackAlgorithm enumeration. For now, the he intent is to preserve the current behaviour.
Comparing reco::TrackBase::TrackAlgorithm enumerators with equality on "magic numbers" and with less/greater than on anything are, in general, considered harmful. This PR replaces these numbers with enumerators, and the less/greater than comparisons with switches with explicit enumerators. In RecoTracker/FinalTrackSelectors/plugins/*ListMerger.cc an ordering between enumerators is genuinely needed, for them a separate array mapping the enumerators for their order "priority" is added (constructed compile-time).
Tested in CMSSW_7_5_X_2015-03-09-0200 with 9k events of RelValTTbar. No regression expected, none observed.
@rovere @VinInn @boudoul @venturia