Fix stub order bug

[tomalin]

This fixes the wrong stub order bug in the tracklet pattern reco code reported in https://github.com/cms-L1TK/cmssw/issues/156 .

The MatchCalculator fix comes from Brent Yates, with an additional correction from Ian Tomalin: (1) to apply fix also to endcap; to apply the fix also to the MatchProcessor. (3) to disable it if multiple stubs per track per layer are allowed in the config;

When running HYBRID with "combined" modules on 1k ttbar200PU events, with DoMultipleMatches = False, the tracking efficiency goes up from 93.71% to 93.96% with this fix.

Also sneaked in unrelated 3-line change to L1TrackNtupleMaker.cc, to update it to inherit from one::EDAnalyzer, since the classic EDAnalyzer will be soon obsolete.

[tschuh]

Since the logic to select the best match is now in MatchCalculator, does that not mean that the logic in Tracklet::addMatch is obsolete and should be removed?

[tomalin]

Since the logic to select the best match is now in MatchCalculator, does that not mean that the logic in Tracklet::addMatch is obsolete and should be removed?

We still need Tracklet::addMatch(), but it calls the Residual class, and I think you're correct that this line can be removed https://github.com/cms-L1TK/cmssw/blob/fixStubOrder/L1Trigger/TrackFindingTracklet/src/Residual.cc#L19 . I'll check.

However, I also realise that we need to make our bug fix not only to MatchCalculator.cc, but also to MatchProcessor.cc, so we're OK when running combined modules.

[tomalin]

PR now includes fix for MatchProcessor.cc too.

[tomalin]

@tschuh I've changed my mind. I think we need to keep https://github.com/cms-L1TK/cmssw/blob/fixStubOrder/L1Trigger/TrackFindingTracklet/src/Residual.cc#L19 . The new "fix" code at https://github.com/cms-L1TK/cmssw/blob/fixStubOrder/L1Trigger/TrackFindingTracklet/src/MatchCalculator.cc#L237 is only executed if the job cfg params set "doMultipleMatches=false". So if instead we run with doMultipleMatches=true, then the fix will not be run. In this case, tracklet->addMatch() will be called for multiple stubs per layer per track, in turning calling multiple times class Residual. This class is only capable of storing one stub per layer per track (corresponding to current FW), so it needs logic to select the one with the smallest residual.

[tomalin]

@aryd could you please explain why MatchCalculator has calls both to tracklet->addMatch() and fullMatches->addMatch()? I'd like to add comment to code explaining this.

[aryd]

@tomalin for historic reasons we store the matches both in the fullMatches memory and as part of the tracklet object. In principle we should be able to remove the tracklet->addMatch(). But it has not been a priority to do this cleanup. I have tried to keep changes minimal for a while to focus on getting the HLS code working.