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] Extend unbalanced OT solvers for more flexibility #539

Merged
merged 18 commits into from
Oct 27, 2023
Merged

[MRG] Extend unbalanced OT solvers for more flexibility #539

merged 18 commits into from
Oct 27, 2023

Conversation

6Ulm
Copy link
Collaborator

@6Ulm 6Ulm commented Oct 25, 2023

Types of changes

  • Modify all unbalanced solvers in unbalanced.py (except for barycenter), so that the argument reg_m can take either a scalar, or an indexable object of length 1 or 2.
  • Edit mm_unbalanced method so that it can be used for both unregularized and regularized problems (previously supported unregularized only). Also, fix an implementation error of matrix $K$ in the $L_2$ case.
  • Fix the KL formula and the corresponding gradient for L-BFGS-B solver.
  • Add corresponding tests to test_unbalanced.py.

Motivation and context / Related issue

  1. The current solvers for unbalanced OT have some limitations, which may restrict their usage in practice.
  • The sinkhorn_unbalanced and lbfgsb_unbalanced methods only allow for reg_m to be a scalar, thus impose the same penalization on both marginals.
  • The mm_unbalanced method has the same limitation and restrict only to the unregularized problem.
  1. The _get_loss_unbalanced method uses incorrect formula of KL divergence, since we are working with unnormalized measures. As a result, the calculation of the gradient is also incorrect.

This PR fixes all of these issues. Moreover, as a byproduct, the new version of sinkhorn_unbalanced method also allows to solve the entropic balanced and semi-relaxed problems.

How has this been tested (if it applies)

Tested when reg_m is a scalar, indexable objects of length 1 and 2 (tuple, list, array/tensor).

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.

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #539 (605a852) into master (e7cba02) will increase coverage by 0.08%.
The diff coverage is 99.34%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #539      +/-   ##
==========================================
+ Coverage   96.38%   96.46%   +0.08%     
==========================================
  Files          67       67              
  Lines       14389    14490     +101     
==========================================
+ Hits        13869    13978     +109     
+ Misses        520      512       -8     

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.

Thanks Huy for the big update that was needed. I have a few comments but it is looking good.

ot/unbalanced.py Outdated Show resolved Hide resolved
ot/unbalanced.py Outdated Show resolved Hide resolved
ot/unbalanced.py Outdated Show resolved Hide resolved
ot/unbalanced.py Outdated Show resolved Hide resolved
ot/unbalanced.py Outdated Show resolved Hide resolved
ot/unbalanced.py Show resolved Hide resolved
ot/unbalanced.py Show resolved Hide resolved
@rflamary rflamary changed the title [WIP] Extend unbalanced OT solvers for more flexibility [MRG] Extend unbalanced OT solvers for more flexibility Oct 27, 2023
@rflamary rflamary merged commit e8767bd into PythonOT:master Oct 27, 2023
13 checks passed
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