JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.6k stars 5.48k forks source link

`sortperm` errors with length of vector less than 4 and if `dims=1`. #55370

Open roflmaostc opened 2 months ago

roflmaostc commented 2 months ago

Hi,

failing on 1.10 and a recent master branch:

julia> sortperm([1,1,1,1], dims=1)
4-element Vector{Int64}:
 1
 2
 3
 4

julia> sortperm([1,1,1], dims=1)
ERROR: MethodError: no method matching sort!(::Vector{…}; alg::Base.Sort.SubArrayOptimization{…}, order::Base.Order.Perm{…}, scratch::Nothing, dims::Int64)
This method does not support all of the given keyword arguments (and may not support any).

Closest candidates are:
  sort!(::AbstractVector{T}, ::Integer, ::Integer, ::Base.Sort.MergeSortAlg, ::Base.Order.Ordering, ::Vector{T}) where T got unsupported keyword arguments "alg", "order", "scratch", "dims"
   @ Base sort.jl:2391
  sort!(::AbstractVector{T}, ::Integer, ::Integer, ::Base.Sort.MergeSortAlg, ::Base.Order.Ordering, ::Union{Nothing, AbstractVector{T}}) where T got unsupported keyword arguments "alg", "order", "scratch", "dims"
   @ Base sort.jl:2393
  sort!(::AbstractVector, ::Integer, ::Integer, ::Base.Sort.Algorithm, ::Base.Order.Ordering, ::Vector) got unsupported keyword arguments "alg", "order", "scratch", "dims"
   @ Base sort.jl:2470
  ...

Stacktrace:
 [1] kwerr(::@NamedTuple{…}, ::Function, ::Vector{…})
   @ Base ./error.jl:174
 [2] _sortperm(A::Vector{…}; alg::Base.Sort.SubArrayOptimization{…}, order::Base.Order.ForwardOrdering, scratch::Nothing, dims::@Kwargs{…})
   @ Base.Sort ./sort.jl:1912
 [3] _sortperm
   @ ./sort.jl:1899 [inlined]
 [4] #sortperm#36
   @ ./sort.jl:1896 [inlined]
 [5] top-level scope
   @ REPL[2]:1
Some type information was truncated. Use `show(err)` to see complete types.

We might have a sensible solution to this. We do the PR later.

Best,

Felix

cc: @sepehr78

LilithHafner commented 2 months ago

There's definitely a bug here.

I think it's that sortperm sometimes does not error on sortperm(::Vector{<:Integer}; dims::Int). None of the other sort methods support sorting vectors with dims set.

roflmaostc commented 2 months ago

Yes, it doesn't error if it goes to this case https://github.com/JuliaLang/julia/blob/065d4567e771af1a642f14c482dd85b3240ec0f3/base/sort.jl#L1907

If dims=1 is explicitly provided, it errors sometimes

LilithHafner commented 2 months ago

This bug is present on 1.9 but not 1.8. In 1.8, both error with

ERROR: MethodError: no method matching sortperm(::Vector{Int64}; dims=1)
Closest candidates are:
  sortperm(::AbstractVector; alg, lt, by, rev, order) at sort.jl:904 got unsupported keyword argument "dims"
  sortperm(::AbstractUnitRange) at range.jl:1384 got unsupported keyword argument "dims"
  sortperm(::AbstractRange) at range.jl:1385 got unsupported keyword argument "dims"
Stacktrace:
 [1] kwerr(::NamedTuple{(:dims,), Tuple{Int64}}, ::Function, ::Vector{Int64})
   @ Base ./error.jl:165
 [2] top-level scope
   @ REPL[1]:1
roflmaostc commented 2 months ago

But it should work by design, no? It's seems like valid call.

LilithHafner commented 2 months ago

I think this was introduced in https://github.com/JuliaLang/julia/pull/45211 by @pcjentsch, @LilithHafner, @oscardssmith