Skip to content

sortperm is unstable #858

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

Closed
tpapp opened this issue Dec 3, 2020 · 1 comment · Fixed by #860
Closed

sortperm is unstable #858

tpapp opened this issue Dec 3, 2020 · 1 comment · Fixed by #860
Labels

Comments

@tpapp
Copy link
Contributor

tpapp commented Dec 3, 2020

?sortperm says

The permutation is guaranteed to be stable even if the sorting algorithm is unstable, meaning that indices of equal elements appear in ascending order.

I am not sure if the algorithm in #773 is meant to be stable, but for StaticArrays v1.0.0,

julia> using StaticArrays

julia> v = [(9, 0.0), (9, 0.0), (9, 0.0), (9, 0.0),
       (3, 0.6000000000000001), (3, 0.6000000000000001)]
6-element Array{Tuple{Int64,Float64},1}:
 (9, 0.0)
 (9, 0.0)
 (9, 0.0)
 (9, 0.0)
 (3, 0.6000000000000001)
 (3, 0.6000000000000001)

julia> sortperm(v; by = last, rev = true)
6-element Array{Int64,1}:
 5
 6
 1
 2
 3
 4

julia> sortperm(SVector(v...); by = last, rev = true)
6-element SArray{Tuple{6},Int64,1,6} with indices SOneTo(6):
 5
 6
 2
 3
 1
 4
@c42f
Copy link
Member

c42f commented Dec 4, 2020

Yes we considered stability at length in #773 and I thought we had it correct. It's pretty subtle though, so I'm not completely surprised if we missed something.

@stev47 do you have time to look at this?

@c42f c42f added the bug label Dec 4, 2020
stev47 added a commit to stev47/StaticArrays.jl that referenced this issue 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 JuliaArrays#858.
stev47 added a commit to stev47/StaticArrays.jl that referenced this issue 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 JuliaArrays#858.
stev47 added a commit to stev47/StaticArrays.jl that referenced this issue 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 JuliaArrays#858.
@c42f c42f closed this as completed in #860 Jan 17, 2021
c42f pushed a commit that referenced this issue Jan 17, 2021
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants