JuliaOptimalTransport / OptimalTransport.jl

Optimal transport algorithms for Julia

https://juliaoptimaltransport.github.io/OptimalTransport.jl/dev

MIT License

94 stars 8 forks source link

Improve `sinkhorn_gibbs` #90

Closed devmotion closed 3 years ago

devmotion commented 3 years ago

This PR improves sinkhorn_gibbs and fixes https://github.com/JuliaOptimalTransport/OptimalTransport.jl/issues/86.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 899809614

Warning: 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.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

59 of 59 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.5%) to 96.623%

Totals
Change from base Build 897834664:	0.5%
Covered Lines:	372
Relevant Lines:	385

💛 - Coveralls

codecov-commenter commented 3 years ago

Codecov Report

Merging #90 (3239f30) into master (7e8cbe2) will increase coverage by 0.54%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #90      +/-   ##
==========================================
+ Coverage   96.07%   96.62%   +0.54%     
==========================================
  Files           3        4       +1     
  Lines         357      385      +28     
==========================================
+ Hits          343      372      +29     
+ Misses         14       13       -1

Impacted Files	Coverage Δ
src/OptimalTransport.jl	`95.70% <100.00%> (+0.50%)`	:arrow_up:
src/utils.jl	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7e8cbe2...3239f30. Read the comment docs.

devmotion commented 3 years ago

It would be great if you could comment on the PR, I think it would be good to fix the AD issue and to improve the sinkhorn implementation.

devmotion commented 3 years ago

What do you think, @zsteve, is it good or should anything be changed?

davibarreira commented 3 years ago

Sorry, I saw that my review was requested, but I haven't been able to do it yet. I still have to take a better look at the code. Feel free to merge without my input, cause I'll probably take a bit longer to review.

zsteve commented 3 years ago

What do you think, @zsteve, is it good or should anything be changed?

This looks good!