Skip to content
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

reverse searchsorted is inconsistent on 1.3 #35272

Closed
rafaqz opened this issue Mar 27, 2020 · 5 comments
Closed

reverse searchsorted is inconsistent on 1.3 #35272

rafaqz opened this issue Mar 27, 2020 · 5 comments
Labels
arrays [a, r, r, a, y, s] bug Indicates an unexpected problem or unintended behavior

Comments

@rafaqz
Copy link
Contributor

rafaqz commented Mar 27, 2020

julia> searchsortedfirst(10:-1:1, 9.5; rev=true)
1

julia> searchsortedfirst(collect(10:-1:1), 9.5; rev=true)
2

julia> searchsortedlast(10:-1:1, 9.5; rev=true)
2

julia> searchsortedlast(collect(10:-1:1), 9.5; rev=true)
1

The range behaviour seems like the correct version, but needless to say it's extremely confusing getting different answers for these.

searchsortedfirst(a, x; by=, lt=, rev=false)

Return the index of the first value in a greater than or equal to x,
according to the specified order. Return length(a) + 1 if x is greater than
all values in a. a is assumed to be sorted.

It gets worse if you use all floats, now both the range and the array are wrong:

julia> searchsortedfirst(10.0:-1.0:1.0, 9.5; rev=true)
2

julia> searchsortedfirst(collect(10.0:-1.0:1.0), 9.5; rev=true)
2

julia> searchsortedlast(10.0:-1.0:1.0, 9.5; rev=true)
1

julia> searchsortedlast(collect(10.0:-1.0:1.0), 9.5; rev=true)
1
julia> versioninfo()
Julia Version 1.3.0
Commit 46ce4d7933 (2019-11-26 06:09 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i5-5300U CPU @ 2.30GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, broadwell)
Environment:
  JULIA_NUM_THREADS = 3
@rafaqz rafaqz changed the title reverse searchsorted is broken for Array on 1.3 reverse searchsorted is inconsistent on 1.3 Mar 27, 2020
@rfourquet
Copy link
Member

The range behaviour seems like the correct version

Actually the array version looks correct to me.

@rfourquet rfourquet added arrays [a, r, r, a, y, s] bug Indicates an unexpected problem or unintended behavior labels Mar 27, 2020
@rafaqz
Copy link
Contributor Author

rafaqz commented Mar 27, 2020

10 is the first value greater than or equal to 9.5. Which has an index of 1.

9 has an index of 2, but its not greater or equal to 9.5.

So 1 is the correct answer for searchsortedfirst

@rfourquet
Copy link
Member

10 is the first value greater than or equal to 9.5

Yes with the usual interpretation of "greater than", but with rev=true, "greater than" means <=, so, in 10:-1:1, 9 is the first value <= 9.5.

@rafaqz
Copy link
Contributor Author

rafaqz commented Mar 27, 2020

That's extremely confusing! The doc should be rewritten, it's probably the reason this is broken.

It really makes it seem like search runs backwards until it finds a number greater than x.

@rfourquet
Copy link
Member

Sure the docs should be inproved, the meaning of rev, by and lt is underspecified in the docstring. In that of searchsorted, it's a bit improved (at least they are mentioned), "... according to the order specified by the by, lt and rev keywords", but you have to know from somewhere else what they mean exactly, it's explained in the docstring of sort!, so there should be at least a link to that function to point where to find the information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants