Skip to content

[MRG] Fix sinkhorn and stabilized barycenters with non-uniform weights - #870

Open
itzzdev09 wants to merge 2 commits into
PythonOT:masterfrom
itzzdev09:barycenter-stabilized-absorption
Open

itzzdev09 wants to merge 2 commits into
PythonOT:masterfrom
itzzdev09:barycenter-stabilized-absorption

Conversation

@itzzdev09

Copy link
Copy Markdown
Contributor

Types of changes

  • Bug fix

Description

With non-uniform weights, ot.bregman.barycenter gives different results depending on method, and only "sinkhorn_log" is correct:

A = np.stack([ot.datasets.make_1D_gauss(15, 4, 2), ot.datasets.make_1D_gauss(15, 10, 2)], 1)
M = ot.utils.dist0(15); M /= M.max()
w = np.array([0.3, 0.7])
lg = ot.bregman.barycenter(A, M, 1e-2, w, method="sinkhorn_log")
np.abs(ot.bregman.barycenter(A, M, 1e-2, w, method="sinkhorn") - lg).max()              # 1.2e-2
np.abs(ot.bregman.barycenter(A, M, 1e-2, w, method="sinkhorn_stabilized") - lg).max()   # 1.8e-1
ot.bregman.barycenter(A, M, 1e-3, np.array([0.5, 0.5]), method="sinkhorn_stabilized").sum()  # 0.599

Evaluating the objective sum_k w_k KL(pi_k | K) confirms that the "sinkhorn_log" result is the minimum (e.g. 33.056 against 33.169 for "sinkhorn"). Two separate problems:

  • barycenter_sinkhorn: the update u <- u * geometricBar(weights, UKv) / UKv leaves the weighted geometric mean of u unchanged, so the barycenter is only optimal if it starts at 1. u was initialised with geometricMean (uniform weights) instead of geometricBar(weights, ...), so with non-uniform weights the iterations converge to a different fixed point. This is a one-line change in the initialisation.
  • barycenter_stabilized: when absorbing, the common scaling max_k v_k is moved into K, but v was then reset to ones for every histogram, which is only right when all histograms have the same v. Also, after an absorption K.T @ u includes exp(beta / reg), which was not taken out of the geometric mean, so q was off by that factor. Now v is divided by the absorbed max, and q is computed with the - beta / reg correction.

After the change the three methods agree to ~1e-11 for all the weights and reg values I tried, and the stabilized barycenter sums to one. The existing test_barycenter only uses alpha = 0.5, and at reg=1e-2 with equal weights the stabilized version happened to be right, which is why these went unnoticed.

How has this been tested

Added test_barycenter_non_uniform_weights (reg 1e-3 and 1e-2, weights [0.2, 0.8]), which checks that all three methods agree and sum to one. It fails on master. test_bregman.py, test/unbalanced, test_da.py and test_utils.py pass.

Checklist

  • I have added tests to cover my changes.
  • I have updated the RELEASES.md file.

@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.88%. Comparing base (98d09a1) to head (375eefb).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #870      +/-   ##
==========================================
+ Coverage   96.86%   96.88%   +0.02%     
==========================================
  Files         128      128              
  Lines       26304    26319      +15     
==========================================
+ Hits        25480    25500      +20     
+ Misses        824      819       -5     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant