Skip to content

[MRG] Free support Sinkhorn barycenters #387

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

Merged
merged 15 commits into from
Jul 27, 2022

Conversation

eddardd
Copy link
Contributor

@eddardd eddardd commented Jul 4, 2022

Types of changes

This PR includes

  • A function in ot.bregman, called free_support_sinkhorn_barycenter, that calculates Wasserstein barycenters using the Sinkhorn solver
  • An example in ./examples/barycenters called plot_free_support_sinkhorn_barycenter.py, illustrating a possible use case of Free Support Sinkhorn Barycenters

Motivation and context / Related issue

The Free Support Wasserstein barycenter already exists in the library (ot.lp.free_support_barycenter). In some applications it may be useful to solve the barycenter problem with entropic regularization.

How has this been tested (if it applies)

A small script has been added to ./examples/barycenters, called plot_free_support_sinkhorn_barycenter.py, illustrating the calculation of the Free Support Sinkhorn Barycenter in a small scale use case.

PR checklist

  • I have read the CONTRIBUTING document.
  • The documentation is up-to-date with the changes I made (check build artifacts).
  • All tests passed, and additional code has been covered with new tests.
  • I have added the PR and Issue fix to the RELEASES.md file.

@eddardd eddardd changed the title Sinkhorn barycenters [MRG] Sinkhorn barycenters Jul 4, 2022
@rflamary rflamary changed the title [MRG] Sinkhorn barycenters [WIP] Sinkhorn barycenters Jul 4, 2022
@codecov
Copy link

codecov bot commented Jul 4, 2022

Codecov Report

Merging #387 (0f2fd24) into master (7c2a952) will decrease coverage by 0.03%.
The diff coverage is 87.09%.

@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
- Coverage   93.80%   93.76%   -0.04%     
==========================================
  Files          23       23              
  Lines        5936     5967      +31     
==========================================
+ Hits         5568     5595      +27     
- Misses        368      372       +4     

@rflamary rflamary changed the title [WIP] Sinkhorn barycenters [WIP] Free support Sinkhorn barycenters Jul 4, 2022
@rflamary
Copy link
Collaborator

rflamary commented Jul 5, 2022

you should use autopep8 to check and correct pep8 mistakes

Copy link
Collaborator

@rflamary rflamary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @eddardd and thank you for the changes. There remain a few things that should be done before merging but it is looking good.

The barycenter of the duck is weird because the regularization is too strong and that one of the problem of entropic barycenter when the two supports are too far, we should keep it that way. I like your example on the 4 distributions so also keep it.

@rflamary
Copy link
Collaborator

Hello @eddardd this is great we are ready for merge IMHO, do you want to add your name website ad contribution to the contributors.md file ?I can wait for you to commit that.

@eddardd
Copy link
Contributor Author

eddardd commented Jul 25, 2022

Hi @rflamary , thanks for your help in adding this new feature to POT. I have been using the library for quite some time, and it feels good to contribute to it! I plan to contributing with a few features (mainly in DA) in the days to come.

I added my info on CONTRIBUTORS.md :)

@rflamary rflamary changed the title [WIP] Free support Sinkhorn barycenters [MRG] Free support Sinkhorn barycenters Jul 27, 2022
@rflamary rflamary merged commit 818c7ac into PythonOT:master Jul 27, 2022
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.

2 participants