-
Notifications
You must be signed in to change notification settings - Fork 16
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
optimisations #28
optimisations #28
Conversation
ensure heap memory gets cleaned by using smart pointers
src/KDTrack.C
Outdated
const double sPhi0 = sin(phi0); | ||
const double ratio = (y0 - b) / (x0 - a); | ||
const double ratio2 = ratio * ratio; | ||
const double errorPhi0Denomitor = (1. / ((y0 - b) * (1 + (ratio2)))); |
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.
DenomiNAtor I guess
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.
of course
} catch (TooManyTracksException& e) { | ||
streamlog_out(MESSAGE) << "caught too many tracks, tightening parameters" << std::endl; | ||
caughtException = true; | ||
factor -= 1; |
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.
how much is the reduction in number of tracks from factor = 10 to factor = 9? Isn't the -= 1 bin too fine?
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 will add a flag to disable the tightening of parameters, and if disabled directly skip the event.
src/KDTrack.C
Outdated
|
||
double debugError2 = sqrt(deltaPhi * deltaPhi * errorRadC * errorRadC + errorDeltaPhi * errorDeltaPhi * radC * radC); | ||
if (debug) | ||
double debugError2 = sqrt(deltaPhi * deltaPhi * errorRadC * errorRadC + errorDeltaPhiSquared * radC * radC); |
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.
minor: since it is a sqrt, why is this named Error2?
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.
the name was there before, but I will rename it
…decreasing refCount
… used in debug only FIXME
fe81137
to
69abf1f
Compare
…if there are too many tracks being created
69abf1f
to
ef66dab
Compare
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 think everything looks ok, and if you get the same performance pre- and post- for the track reconstruction efficiency then I'd go ahead and merge it .
See also AIDASoft/aidaTT#19
BEGINRELEASENOTES
ENDRELEASENOTES