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

Release 0.10.1 broke pairwise haversine #210

Closed
briochemc opened this issue Jan 29, 2021 · 3 comments · Fixed by #211
Closed

Release 0.10.1 broke pairwise haversine #210

briochemc opened this issue Jan 29, 2021 · 3 comments · Fixed by #211

Comments

@briochemc
Copy link

briochemc commented Jan 29, 2021

The latest release (v0.10.1) broke a simple pairwise call with the Haversine distance that used to work with (v0.10.0):

pairwise(Haversine(1), rand(3,2), dims=1)

I'm not sure what changed, but I guess something went wrong, and the example above could be added in the test suite to ensure this does not happen again?

Below are complete MWEs:

  • using v0.10.0:

    (@v1.5) pkg> activate --temp
     Activating new environment at `/var/folders/75/bdw1v8j10w9dbmvjmc13fkj80000gn/T/jl_1MvJRQ/Project.toml`
    
    (jl_1MvJRQ) pkg> add Distances@v0.10.0
       Updating registry at `~/.julia/registries/General`
       Updating git-repo `https://github.com/JuliaRegistries/General.git`
      Resolving package versions...
    Updating `/private/var/folders/75/bdw1v8j10w9dbmvjmc13fkj80000gn/T/jl_1MvJRQ/Project.toml`
      [b4f34e82] + Distances v0.10.0
    Updating `/private/var/folders/75/bdw1v8j10w9dbmvjmc13fkj80000gn/T/jl_1MvJRQ/Manifest.toml`
      [b4f34e82] + Distances v0.10.0
      [8f399da3] + Libdl
      [37e2e46d] + LinearAlgebra
      [9a3f8284] + Random
      [9e88b42a] + Serialization
      [2f01184e] + SparseArrays
      [10745b16] + Statistics
    
    julia> using Distances
    
    julia> pairwise(Haversine(1), rand(3,2), dims=1)
    3×3 Array{Float64,2}:
     0.0         0.00649202  0.00718727
     0.00649202  0.0         0.0129741
     0.00718727  0.0129741   0.0
  • and using v0.10.1:

    (@v1.5) pkg> activate --temp
     Activating new environment at `/var/folders/75/bdw1v8j10w9dbmvjmc13fkj80000gn/T/jl_kXGxKf/Project.toml`
    
    (jl_kXGxKf) pkg> add Distances@v0.10.1
       Updating registry at `~/.julia/registries/General`
       Updating git-repo `https://github.com/JuliaRegistries/General.git`
      Resolving package versions...
    Updating `/private/var/folders/75/bdw1v8j10w9dbmvjmc13fkj80000gn/T/jl_kXGxKf/Project.toml`
      [b4f34e82] + Distances v0.10.1
    Updating `/private/var/folders/75/bdw1v8j10w9dbmvjmc13fkj80000gn/T/jl_kXGxKf/Manifest.toml`
      [b4f34e82] + Distances v0.10.1
      [8f399da3] + Libdl
      [37e2e46d] + LinearAlgebra
      [9a3f8284] + Random
      [9e88b42a] + Serialization
      [2f01184e] + SparseArrays
      [10745b16] + Statistics
    
    julia> using Distances
    
    julia> pairwise(Haversine(1), rand(3,2), dims=1)
    ERROR: ArgumentError: expected both inputs to have length 2 in Haversine{Int64}(1) distance
    Stacktrace:
    [1] haversine_error(dist::Haversine{Int64})
      @ Distances ~/.julia/packages/Distances/glUtY/src/haversine.jl:34
    
    [2] Haversine(x::Float64, y::Float64)
      @ Distances ~/.julia/packages/Distances/glUtY/src/haversine.jl:17
    
    [3] result_type(f::Haversine{Int64}, a::Type{T} where T, b::Type{T} where T)
      @ Distances ~/.julia/packages/Distances/glUtY/src/generic.jl:36
    
    [4] result_type(dist::Haversine{Int64}, a::Array{Float64,2}, b::Array{Float64,2})
      @ Distances ~/.julia/packages/Distances/glUtY/src/generic.jl:35
    
    [5] #pairwise#5(dims::Int64, ::typeof(pairwise), metric::Haversine{Int64}, a::Array{Float64,2})
      @ Distances ~/.julia/packages/Distances/glUtY/src/generic.jl:328
    
    [6] top-level scope
      @ REPL[7]:1
@briochemc briochemc changed the title Release 10.1 breaking simple data by row pairwise Release 0.10.1 broke pairwise (by row) Jan 29, 2021
@briochemc
Copy link
Author

Not sure, but it looks like the issue is with the new generic return_type function, which I think assumes that any distance can work in 1D space, i.e., take scalars like dist(a::Number, b::Number), while haversine always expects 2-element arrays that live in a 2D space?

@briochemc briochemc changed the title Release 0.10.1 broke pairwise (by row) Release 0.10.1 broke pairwise haversine Jan 29, 2021
@dkarrasch
Copy link
Member

😭 I'm very sorry! Apparently, we don't have any pairwise tests for Haversine, so this didn't come up in the process. This is closely related to #209 in that this used to be handled by the Float64 fallback.

@briochemc
Copy link
Author

No worries! I had a quick look at the code and just tried to decipher the problem to help you speed up the recovery process! :)

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 a pull request may close this issue.

2 participants