DR: Excluding Duplicates for CM #267
Conversation
…acks excluding duplicates
|
@dally96 given that in Settings.h , int numTracksComparedPerBin_ remains 9999, this PR should have no effect on tracking performance. But it fails git CI because the tracking performance has got worse. Do you understand why? |
|
I think it might have to do with the fact that this code doesn’t overwrite the inputstublists when a track is merged. For example, with this code, if track 1 is a duplicate of track 0, track 1’s stubs do not get added to track 0’s stub list, but track 1 will not be compared to the other tracks. So if track 2 is a duplicate of track 1, it won’t be removed. We tried overwriting the stublist before but it led to an error in the getPhiRes function in PurgeDuplicate that we didn’t figure out.
… On Apr 2, 2024, at 11:52 PM, Ian Tomalin ***@***.***> wrote:
@dally96 <https://github.com/dally96> given that in Settings.h , int numTracksComparedPerBin_ remains 9999, this PR should have no effect on tracking performance. But it fails git CI because the tracking performance has got worse. Do you understand why?
—
Reply to this email directly, view it on GitHub <#267 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ARUNMFZHRZLOIYQPXP77WE3Y3MSB3AVCNFSM6AAAAABFORV2GWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZTGE3DONZTGI>.
You are receiving this because you were mentioned.
|
|
Does the current code, before this PR, also behave incorrectly, by not overwriting the inputstublists when two tracks are merged? |
|
In your talk, you stated that if you correct this PR, so it does overwrite the inputstublists, then it crashes. My guess is that Tracklet:::proj() which you are calling from PurgeDuplicate is being called with a layer that fails the assert statement in Tracklet::validProj(). |
There are two loops one to check if two tracks are duplicate and the other to merge the tracks if they are duplicates. In the loop to merge tracks, the stubs and stub ids do get merged, but after all the comparisons have been done. This error arises when I try to use the merged stubs for the comparison (although they should really be in one loop for this PR, I will fix that, but the same error persists). |
|
There's been no progress on this PR in 4 months. Shall we close it? |
…te tracks are skipped in the comparison
| const unsigned int curSeed = aTrack->seedIndex(); | ||
| static const std::vector<int> ranks{1, 5, 2, 7, 4, 3, 8, 6}; | ||
| unsigned int curSeed = aTrack->seedIndex(); | ||
| std::vector<int> ranks{1, 5, 2, 7, 4, 3, 8, 6}; |
There was a problem hiding this comment.
Why has the "static const" qualifier been removed here?
| @@ -181,38 +187,70 @@ void PurgeDuplicate::execute(std::vector<Track>& outputtracks, unsigned int iSec | |||
| if (inputtracklets_.empty()) | |||
| continue; | |||
| const unsigned int numStublists = inputstublists_.size(); | |||
| std::vector<int> seedRankIdx(numStublists); | |||
There was a problem hiding this comment.
Add comment that you're ordering the tracks by seed rank.
| @@ -86,6 +87,11 @@ void PurgeDuplicate::execute(std::vector<Track>& outputtracks, unsigned int iSec | |||
| inputstublists_.clear(); | |||
| mergedstubidslists_.clear(); | |||
|
|
|||
| sortedinputtracklets_.clear(); | |||
There was a problem hiding this comment.
I've never understood why all these vectors are declared as class data members. If they were instead declared inside the loop over bins, e.g. near here https://github.com/cms-L1TK/cmssw/blob/dally_CMFix/L1Trigger/TrackFindingTracklet/src/PurgeDuplicate.cc#L140 , they would automatically be deleted and recreated between loops, so no clear would be needed either here nor near L410. Am I missing something?
|
This fails git CI only because the performance of HYBRID_DISPLACED is worse https://gitlab.cern.ch/cms-l1tk/cmssw_CI/-/pipelines/8056886 . The reason it is worse is presumably your discovery that that the DR binning equations need changing to work for displaced tracks. |
Fixed track comparison in PurgeDuplicate.cc so that the comparison modules ignore duplicate tracks. However, this does not overwrite the stub list if a duplicate is found and is merged.