Skip to content

sort: avoid use of ReverseOrdering for BitonicSort #860

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

Merged
merged 1 commit into from
Jan 17, 2021

Conversation

stev47
Copy link
Contributor

@stev47 stev47 commented Dec 4, 2020

We reverse the order as part of the bitonic sort algorithm, but
ReverseOrdering is not the true reverse order for an order ::Perm.
This led to non-stable sorting for sortperm.

Fixed by hand-wiring the reversal.
Fixes #858.

We reverse the order as part of the bitonic sort algorithm, but
`ReverseOrdering` is not the true reverse order for an order `::Perm`.
This led to non-stable sorting for `sortperm`.

Fixed by hand-wiring the reversal.
Fixes JuliaArrays#858.
@stev47 stev47 force-pushed the fix/sortperm-stable branch from 93a5a41 to 59a924d Compare December 4, 2020 14:37
@stev47
Copy link
Contributor Author

stev47 commented Dec 4, 2020

I'm not sure why codecov/project is failing with -0.01% for this simple pull request and I'm not eager to give codecov access to my github data in order to be able to view that detailed report.

@mateuszbaran
Copy link
Collaborator

Total coverage decreased because after this patch there is fewer covered lines (you've removed two lines that were considered as covered), nothing to worry about. There are no new non-covered lines.

Copy link
Collaborator

@mateuszbaran mateuszbaran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mateuszbaran
Copy link
Collaborator

Can we merge this and tag 1.0.3?

@c42f c42f merged commit 4b09eab into JuliaArrays:master Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sortperm is unstable
3 participants