-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Speed-up seed/dup. cleaning mkFit #37122
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
Speed-up seed/dup. cleaning mkFit #37122
Conversation
This was broken as struct/union with raw unsigned construct in struct binnor::B_pair got removed during CMSSW review process. 'raw' was actually used for sorting but this never showd up as the templates did not get instantianted, i.e., binnor was not used in cmssw. Here we remove explicit bin1/2 variables with specified bit-size and replace them with a single unsigned packed_value and provide hand-written bit-shifting / masking for bin-data access.
…(template type X) to start with a clearer from_X_...
Remove commented-out demonstration code.
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37122/28645
|
|
A new Pull Request was created by @leonardogiannini for master. It involves the following packages:
@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8de885/22785/summary.html Comparison SummarySummary:
|
|
@cms-sw/reconstruction-l2 |
|
urgent
|
|
assign @cms-sw/tracking-pog-l2 |
Seems like the tag didn't work. |
It would need to be without |
@clacaputo 100 evts with TTbar PU 50 |
|
assign tracking-pog-l2 |
|
+reconstruction
|
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
assign tracking-pog |
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.
not necessarily needing to be followed-up.
|
|
||
| axis_pow2_u1<float, unsigned short, 16, 8> ax_phi(-Const::PI, Const::PI); | ||
| axis<float, unsigned short, 8, 8> ax_eta(-3.0, 3.0, 30u); | ||
| binnor<unsigned int, decltype(ax_phi), decltype(ax_eta), 24, 8> phi_eta_binnor(ax_phi, ax_eta); |
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.
usual set of hard-coded constants.
| continue; | ||
| int a = trk.getHitLyr(i); | ||
| int b = trk.getHitIdx(i); | ||
| hitMap.insert(std::make_pair(b * 1000 + a, i > 0 ? itrack : -itrack)); //negative for first hit in trk |
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.
what's 1000?
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.
a way to write layer and hit id in a single int.
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.
I know, I commented already here: #36546 (comment), not sure if it's in the plans to document it.
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
The seed cleaning algorithm (used in initial, high pT triplet, detached quad, and triplet) and the duplicate cleaning used in the pixelLess iteration are updated to reduce the processing time, while the physics is unchanged.
the seed cleaning is updated from an N^2 comparison of all the pairs to a comparison of the tracks that can be approximately
compatible for removal only. The tracks are binned in phi and eta and for each track, the comparison is made only inside compatible bins
the duplicate cleaning consisted of a nested loop comparing hits. The new algorithm uses 2 simple loops per track: 1 to map hits to tracks and the second to find all the tracks sharing hits with the one examined (similarly to TrajectoryCleanerBySharedHits)
The seed cleaning update is described here:
Jan 31 update
The duplicate cleaning is mentioned here (p.23):
Feb 14 update
PR validation:
The physics is validated with TTbar PU50, where blue is before PR and red is after PR
http://legianni.web.cern.ch/legianni/plots-PR37122/
CPU profiling with igprof can be found here
global
seed cleaning
dup cleaning
global
seed cleaning
dup cleaning