Add wasserstein and squared2wasserstein
#91
Conversation
Pull Request Test Coverage Report for Build 897793775
💛 - Coveralls |
| @@ -17,10 +17,13 @@ export emd, emd2 | |||
| export sinkhorn_stabilized, sinkhorn_stabilized_epsscaling, sinkhorn_barycenter | |||
| export sinkhorn_unbalanced, sinkhorn_unbalanced2 | |||
| export quadreg | |||
| export ot_cost, ot_plan | |||
| export ot_cost, ot_plan, wasserstein, squared2wasserstein | |||
|
|
|||
There was a problem hiding this comment.
Why do you think a squared2wasserstein is necessary? Why not only wasserstein? The ot_cost would already calculate the squared case if the user wished so, or he could just square the output of the `wasserstein
There was a problem hiding this comment.
The squared version is useful when working with e.g. normal distributions. And it is quite expensive to first take the square root and then square the result again. Therefore also e.g. Distances contains a dedicated SqEuclidean distance and Distributions a pdfL2squared.
src/exact.jl
Outdated
|
|
||
| See also: [`ot_cost`](@ref) | ||
| """ | ||
| ot_plan |
There was a problem hiding this comment.
Why defining this docstring like this? Isn't it redundant since a very similar docstring is present in the following functions?
There was a problem hiding this comment.
The main motivation is to explain what ot_cost and ot_plan are designed for without focusing on the special cases such as e.g. for univariate distributions or discrete 1D measures. The common practice is to only define docstrings for the special cases if they are really necessary and e.g. have special keyword arguments or return types.
There was a problem hiding this comment.
I see. Perhaps in another PR, we should reduce some of the repeated info in the other repeated functions.
test/wasserstein.jl
Outdated
| ν = Normal(randn(), randexp()) | ||
| for p in (1, 2, 3, randexp()), metric in (Euclidean(), TotalVariation()) | ||
| # without additional keyword arguments | ||
| w = wasserstein(μ, ν; p=Val(p), metric=metric) |
There was a problem hiding this comment.
is it necessary to use p=Val(p) when using the wasserstein function?
There was a problem hiding this comment.
Yes, otherwise one can't dispatch on p to use optimized cost functions such as SqEuclidean.
There was a problem hiding this comment.
Hmm. I understand, although it's a bit unfortunate, cause it's a bit odd from a user stand point.
There was a problem hiding this comment.
In the latest commit I added support for Real-valued ps and a note in the docstring that explains that sometimes Val(p) might be more efficient.
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
==========================================
+ Coverage 95.91% 96.07% +0.16%
==========================================
Files 2 3 +1
Lines 343 357 +14
==========================================
+ Hits 329 343 +14
Misses 14 14
Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build 896480278Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 897793775Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 896486751Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 897702779Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Add
wassersteinandsquared2wassersteinfunctions that compute the p-Wasserstein distance and the squared 2-Wasserstein distance.#88 and #85 will add support for more types of marginals.