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

[MRG] Generalized Wasserstein Barycenters #372

Merged
merged 9 commits into from
May 6, 2022

Conversation

eloitanguy
Copy link
Collaborator

@eloitanguy eloitanguy commented May 5, 2022

Types of changes

  • Added a generalized_free_support_barycenter solver in ot/lp/__init__.py (next to free_support_barycenter)
  • Added an associated example examples/barycenters/plot_generalized_free_support_barycenter.py
  • Fixed doc for free_support_barycenter (mis-numbered/missing references + added details on variable names)
  • Fixed a reproducibilty issue in a test for a partial gromov solver

Motivation and context / Related issue

Implements the article "Generalised Wasserstein Barycenter" [42]

[42] DELON, Julie, GOZLAN, Nathael, et SAINT-DIZIER, Alexandre.
Generalized Wasserstein barycenters between probability measures living on different subspaces.
arXiv preprint arXiv:2105.09755, 2021.

How has this been tested (if it applies)

Added tests in tests_ot.py, accounting for dimensionality, optional arguments and backends

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.

@rflamary rflamary changed the title Generalized Wasserstein Barycenters [WIP] [WIP] Generalized Wasserstein Barycenters May 5, 2022
@codecov
Copy link

codecov bot commented May 5, 2022

Codecov Report

Merging #372 (bc6534c) into master (eccb138) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #372      +/-   ##
==========================================
+ Coverage   93.78%   93.85%   +0.07%     
==========================================
  Files          23       23              
  Lines        5902     5926      +24     
==========================================
+ Hits         5535     5562      +27     
+ Misses        367      364       -3     

@rflamary rflamary merged commit ccc076e into PythonOT:master May 6, 2022
@rflamary rflamary changed the title [WIP] Generalized Wasserstein Barycenters [MRG] Generalized Wasserstein Barycenters May 6, 2022
@@ -137,6 +137,7 @@ def test_partial_wasserstein():


def test_partial_gromov_wasserstein():
np.random.seed(42)
Copy link
Collaborator

@agramfort agramfort May 8, 2022

Choose a reason for hiding this comment

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

@eloitanguy @rflamary you should not use global seeds in tests. It makes tests coupled as they share a random state. It may happen that a test that does not control its seed fails only if run after another test and it makes debugging a nightmare. You should np.random.default_rng these days.

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