-
Notifications
You must be signed in to change notification settings - Fork 27
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
ENH add an introsort implementation #7
base: main
Are you sure you want to change the base?
Conversation
|
||
function sort!(v::AbstractVector, lo::Int, hi::Int, a::IntroSortAlg, o::Ordering) | ||
N = length(v) | ||
if N > 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.
this guard should rather be on hi - lo
> 1 (which can cause an exception due to log2(...) == -Inf).
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.
Hmmm, I wonder what this comment meant...
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.
It's referring to the line below, and I think that's right. Care to submit a pull request?
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.
So this line should just be hi-li > 1
, will see if that works and push an update commit here.
Any thoughts on optimizations for index_of_median_of_three
?
What's the advantage of introsort? |
I haven't looked at this yet, but generally, introsort uses quick sort, It's a good algorithm to have available. On Monday, October 27, 2014, Steven G. Johnson notifications@github.com
|
it's been more than 5 years. close it, merge it, or start another repo |
There's an unresolved comment above that somebody needs to address (and also needs a rebase). (BTW, necro-posting generally isn't useful if you're not volunteering to work on PRs yourself.) |
This is an implementation from the pseudo-code in Musser's paper.
I haven't done any performance testing, no doubt there is optimization to do here e.g.
@inbounds
and something better forindex_of_median_of_three
(which atm is terrible)...