-
Notifications
You must be signed in to change notification settings - Fork 6
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
MP fix new tracklet with projections #237
Conversation
89acaf4
to
b4579ab
Compare
CI is failing due to the |
I'm confused. A tracklet can only have one projection to a specific layer or disk. I don't see how we can have multiple projections to the same layer with one tracklet. |
Sorry, I mislabeled it above since I was testing |
@bryates I think we're just waiting for you to finish answering Anders's comment above, before merging this. |
Ok
@aryd, running again with it just set to print
So with the old setup, The corresponding FM for Looking into |
Can you share the code for how you generated the printout? In the emulation the data is passed directly as objects without out writing/reading files. Files can be written out for use in the HLS code. |
I've just pushed a branch called cmssw/L1Trigger/TrackFindingTracklet/src/MatchProcessor.cc Lines 833 to 840 in e8e9db8
|
I looked at the code in mp_log, but it was not obvious where this: tracklet = 0x1dd0b000 was printed out - can you tell men the line numbers for these printouts? (I did not yet try to checkout and run the code.) |
These lines should give a similar printout: cmssw/L1Trigger/TrackFindingTracklet/src/MatchProcessor.cc Lines 837 to 839 in e8e9db8
it will print whether it would be skipped, the projection, the tracklet in parentheses, and the previous projection It's currently set to print just cmssw/L1Trigger/TrackFindingTracklet/src/MatchProcessor.cc Lines 833 to 834 in e8e9db8
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine now. (We had a long exchange to get to the bottom of this, but I think the proposed fix is good.)
* Check for new proj instead of new tracklet * Ran code-format * Back to tracklet, bool for first proj --------- Co-authored-by: bryates <brent.yates@email.ucr.edu>
* Check for new proj instead of new tracklet * Ran code-format * Back to tracklet, bool for first proj --------- Co-authored-by: bryates <brent.yates@email.ucr.edu>
* Check for new proj instead of new tracklet * Ran code-format * Back to tracklet, bool for first proj --------- Co-authored-by: bryates <brent.yates@email.ucr.edu>
PR description:
This PR fixes a disagreement between the HLS and emulation. Currently, the emulation resets the best match values only when a new
tracklet
is found. This means tracklets with multiple projections can have issues, e.g.:The PR looks instead uses the current and previous
projection
. It currently uses the string value, as this was easy to access. If this is inefficient, we could look into comparing the member values.PR validation:
Full agreement with HLS in the barrel and disks for
PHIB
andPHIC