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

introduce ent_reg_cost / kl_reg_cost in Sinkhorn to clarify which is returned to the user in SinkhornOutput objects. #376

Merged
merged 7 commits into from
Jun 26, 2023

Conversation

marcocuturi
Copy link
Contributor

@marcocuturi marcocuturi commented Jun 26, 2023

Currently, the Sinkhorn algorithm outputs the value reg_ot_cost, which is currently computed as the KL regularized OT problem's objective value.

This can lead to confusion, since Sinkhorn is often described as "entropy regularized". The two are equivalent up to the addition of the entropies of the two marginal distributions a and b. This confusion is illustrated in, e.g. #361, at the end of the definition of the Monge gap, where @theouscidda6 had to correct reg_ot_cost by substracting again these 2 entropies to output the entropy regularized OT objective.

This PR clarifies this by introducing the two closely related ent_reg.. and kl_reg... quantities, knowing that at the moment the "default" reg_ot_cost is kl_reg_cost.

Note: It might be worth deprecating reg_ot_cost because of its ambiguity.

@marcocuturi marcocuturi changed the title introduce ent_reg / kl_reg to clarify introduce ent_reg_cost / kl_reg_cost in Sinkhorn to clarify which is given to the user. Jun 26, 2023
@marcocuturi marcocuturi changed the title introduce ent_reg_cost / kl_reg_cost in Sinkhorn to clarify which is given to the user. introduce ent_reg_cost / kl_reg_cost in Sinkhorn to clarify which is returend to the user in SinkhornOutput objects. Jun 26, 2023
@marcocuturi marcocuturi changed the title introduce ent_reg_cost / kl_reg_cost in Sinkhorn to clarify which is returend to the user in SinkhornOutput objects. introduce ent_reg_cost / kl_reg_cost in Sinkhorn to clarify which is returned to the user in SinkhornOutput objects. Jun 26, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2023

Codecov Report

Merging #376 (b75d390) into main (05c0afb) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #376      +/-   ##
==========================================
+ Coverage   88.49%   88.51%   +0.01%     
==========================================
  Files          52       52              
  Lines        5651     5659       +8     
  Branches      837      839       +2     
==========================================
+ Hits         5001     5009       +8     
  Misses        530      530              
  Partials      120      120              
Impacted Files Coverage Δ
src/ott/solvers/linear/sinkhorn_lr.py 97.74% <ø> (ø)
src/ott/solvers/linear/sinkhorn.py 96.93% <100.00%> (+0.07%) ⬆️

src/ott/solvers/linear/sinkhorn.py Outdated Show resolved Hide resolved
src/ott/solvers/linear/sinkhorn.py Outdated Show resolved Hide resolved
src/ott/solvers/linear/sinkhorn.py Outdated Show resolved Hide resolved
src/ott/solvers/linear/sinkhorn.py Outdated Show resolved Hide resolved
@michalk8
Copy link
Collaborator

Thanks @marcocuturi , merging this!

@michalk8 michalk8 merged commit 5fa481a into main Jun 26, 2023
@michalk8 michalk8 deleted the mott2 branch June 26, 2023 22:13
@michalk8 michalk8 mentioned this pull request Jun 26, 2023
marcocuturi added a commit that referenced this pull request Jun 27, 2023
* pydocs: more precise unbalanced

* typo

* typo
@ott-jax ott-jax deleted a comment from michalk8 Jul 1, 2023
michalk8 pushed a commit that referenced this pull request Jun 27, 2024
… is returned to the user in `SinkhornOutput` objects. (#376)

* introduce ent_reg / kl_reg to clarify

* pydocs corrections.

* flax `submodules` spellcheck issue

* move submodules to technical.txt

* move to technical.txt

* minor fixes

* Polish docs
michalk8 pushed a commit that referenced this pull request Jun 27, 2024
* pydocs: more precise unbalanced

* typo

* typo
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