-
-
Notifications
You must be signed in to change notification settings - Fork 69
Adds descending sort order to quicksort, quickselect, and partial sort #140
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
Conversation
70c4bce
to
a5bbb62
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.
Looks good so far, yet to review the changes in src
files. I simplified the code in scalar function to make it more readable. Let me know if that looks good.
This reverts commit 9d508c1.
4e41904
to
f19214a
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.
LGTM, thanks Matthew!
After this change, qsort, qselect, and partial_qsort now take an additional argument to select descending order. This change also adds tests for this functionality.
For benchmarking, I compared the ascending direction against the previous version, and then compared the ascending direction against the new descending direction
When we compare the ascending and descending orders against once another using quicksort, we see no significant change (see the full results at the bottom).
Comparing to previous versions, there is no significant effect on the performance of quickselect, and we see a small speed up for 32-bit values using quicksort
Quickselect:
Quicksort
Ascending/descending comparison: