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

Fix negative values in the pairwise computations of SqMahalanobis #180

Merged
merged 3 commits into from
Oct 1, 2020

Conversation

devmotion
Copy link
Member

Fixes negative values in the pairwise computations with SqMahalanobis, in the same way as done for the (weighted) (Sq)Euclidean distances (#161) and CosineDist.

On master:

julia> using Distances

julia> a = [0.3 0.3 + eps()]
1×2 Array{Float64,2}:
 0.3  0.3

julia> pairwise(SqMahalanobis(ones(1, 1)), a; dims = 2)
2×2 Array{Float64,2}:
  0.0          -2.77556e-17
 -2.77556e-17   0.0

julia> pairwise(SqMahalanobis(ones(1, 1)), a, a; dims = 2)
2×2 Array{Float64,2}:
  0.0          -2.77556e-17
 -2.77556e-17   0.0

With this PR:

julia> using Distances

julia> a = [0.3 0.3 + eps()]
1×2 Array{Float64,2}:
 0.3  0.3

julia> pairwise(SqMahalanobis(ones(1, 1)), a; dims = 2)
2×2 Array{Float64,2}:
 0.0  0.0
 0.0  0.0

julia> pairwise(SqMahalanobis(ones(1, 1)), a, a; dims = 2)
2×2 Array{Float64,2}:
 0.0  0.0
 0.0  0.0

@devmotion devmotion changed the title Fix non Fix negative values in the pairwise computations of SqMahalanobis Sep 29, 2020
@devmotion
Copy link
Member Author

Test error https://travis-ci.org/github/JuliaStats/Distances.jl/jobs/731222246 seems to be unrelated

@dkarrasch
Copy link
Member

Test error https://travis-ci.org/github/JuliaStats/Distances.jl/jobs/731222246 seems to be unrelated

True, but just to make sure...

@dkarrasch dkarrasch closed this Sep 29, 2020
@dkarrasch dkarrasch reopened this Sep 29, 2020
@devmotion
Copy link
Member Author

Tests pass now 🎉

@dkarrasch dkarrasch merged commit f97709f into JuliaStats:master Oct 1, 2020
@devmotion devmotion deleted the sqmahalanobis branch October 1, 2020 08:36
@devmotion
Copy link
Member Author

devmotion commented Oct 1, 2020

Is it possible to make a new release with this fix (I already updated the version number)? 🙂 Some tests in KernelFunctions are broken due to this problem.

@nalimilan
Copy link
Member

Already done by @dkarrasch! JuliaRegistries/General#22239

@devmotion
Copy link
Member Author

Oh great, somehow the comments to the commit didn't show up on my phone. Sorry for the noise!

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 this pull request may close these issues.

3 participants