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

[WIP] Restructure unbalanced OT module #658

Merged
merged 25 commits into from
Sep 10, 2024
Merged

Conversation

6Ulm
Copy link
Collaborator

@6Ulm 6Ulm commented Jul 16, 2024

Types of changes

This PR aims to improve the current module unbalanced.py on two aspects:

  • The current module contains 3 seperate groups of unbalanced OT solvers: Sinkhorn, Majorization-Minimization and LBFGS-B. It is preferable to break these groups into seperate files/modules for easier maintenance.
  • In the current Sinkhorn solver, the reference measure used in the regulazation is always fixed as $a b^T$ (for reg_type=kl) or one-matrix (for reg_type=entropy). With trivial modification, we can allow for any nonnegative reference measure. This will be useful for the ot.solve, since we consider a very generic unbalanced OT problem.

Major changes:

  • In case reg_type=kl, allow for any reference measure c in the regularization term ($c = ab^T$ by default)
  • Add ot.unbalanced.lbfgsb_unbalanced2, which returns the UOT loss. This is consistent with other solvers in POT.

Motivation and context / Related issue

How has this been tested (if it applies)

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.

Copy link

codecov bot commented Jul 17, 2024

Codecov Report

Attention: Patch coverage is 99.30556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.88%. Comparing base (bd56809) to head (ad2e6f8).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #658      +/-   ##
==========================================
+ Coverage   96.85%   96.88%   +0.03%     
==========================================
  Files          88       93       +5     
  Lines       17910    18166     +256     
==========================================
+ Hits        17346    17600     +254     
- Misses        564      566       +2     

Copy link
Collaborator

@cedricvincentcuaz cedricvincentcuaz left a comment

Choose a reason for hiding this comment

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

Thank you @6Ulm for this nice cleaning and new features. Follows some comments and you forgot to update the release.md

ot/unbalanced/__init__.py Outdated Show resolved Hide resolved
ot/unbalanced/_lbfgs.py Outdated Show resolved Hide resolved
ot/unbalanced/_lbfgs.py Show resolved Hide resolved
ot/unbalanced/_lbfgs.py Show resolved Hide resolved
ot/unbalanced/_lbfgs.py Outdated Show resolved Hide resolved
ot/unbalanced/_mm.py Outdated Show resolved Hide resolved
ot/unbalanced/_mm.py Show resolved Hide resolved
ot/unbalanced/_sinkhorn.py Outdated Show resolved Hide resolved
ot/unbalanced/_sinkhorn.py Outdated Show resolved Hide resolved
ot/unbalanced/_sinkhorn.py Show resolved Hide resolved
Copy link
Collaborator

@cedricvincentcuaz cedricvincentcuaz left a comment

Choose a reason for hiding this comment

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

Great, thank your for your updates @6Ulm.

@cedricvincentcuaz cedricvincentcuaz merged commit 2aa8338 into PythonOT:master Sep 10, 2024
19 of 20 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.

3 participants