Skip to content

Commit 59a924d

Browse files
committed
sort: avoid use of ReverseOrdering for BitonicSort
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.
1 parent 0e43102 commit 59a924d

File tree

2 files changed

+5
-4
lines changed

2 files changed

+5
-4
lines changed

src/sort.jl

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import Base.Order: Forward, Ordering, Perm, ReverseOrdering, ord
1+
import Base.Order: Forward, Ordering, Perm, ord
22
import Base.Sort: Algorithm, lt, sort, sortperm
33

44

@@ -51,8 +51,7 @@ _sort(a::NTuple, alg, order) = sort!(Base.copymutable(a); alg=alg, order=order)
5151
function swap_expr(i, j, rev)
5252
ai = Symbol('a', i)
5353
aj = Symbol('a', j)
54-
order = rev ? :revorder : :order
55-
return :( ($ai, $aj) = @inbounds lt($order, $ai, $aj) ? ($ai, $aj) : ($aj, $ai) )
54+
return :( ($ai, $aj) = @inbounds lt(order, $ai, $aj) $rev ? ($ai, $aj) : ($aj, $ai) )
5655
end
5756

5857
function merge_exprs(idx, rev)
@@ -83,7 +82,6 @@ _sort(a::NTuple, alg, order) = sort!(Base.copymutable(a); alg=alg, order=order)
8382
symlist = (Symbol('a', i) for i in idx)
8483
return quote
8584
@_inline_meta
86-
revorder = Base.Order.ReverseOrdering(order)
8785
($(symlist...),) = a
8886
($(sort_exprs(idx)...);)
8987
return ($(symlist...),)

test/sort.jl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ using StaticArrays, Test
2525
vp = sortperm(v)
2626
@test v[vp] == sort(v)
2727
end
28+
29+
# stability
30+
@test sortperm(SA[1, 1, 1, 0]) == SA[4, 1, 2, 3]
2831
end
2932

3033
end

0 commit comments

Comments
 (0)