Skip to content

[MRG] Fix proximal batch plan weighting - #843

Closed
ANONYMOUSZED-beep wants to merge 1 commit into
PythonOT:masterfrom
ANONYMOUSZED-beep:fix-842-proximal-plan-sign
Closed

[MRG] Fix proximal batch plan weighting#843
ANONYMOUSZED-beep wants to merge 1 commit into
PythonOT:masterfrom
ANONYMOUSZED-beep:fix-842-proximal-plan-sign

Conversation

@ANONYMOUSZED-beep

Copy link
Copy Markdown

Types of changes

  • Bug fix
  • Regression tests
  • Documentation/release note

Motivation and context / Related issue

Closes #842.

proximal_bregman_log_plan_batch documented the proximal subproblem with the term C - inner_reg * log(T), but constructed both the iterative and last_step kernels with C + inner_reg * log(T). That inverted the previous transport plan instead of weighting the next kernel by it.

This changes both paths to:

K_proj = -(C - inner_reg * log_T) / (reg + inner_reg)

The docstring now uses the same formula. The regression tests use a 2x2 log cross-ratio, where row and column Sinkhorn scalings cancel, to directly verify the recurrence for the ordinary and differentiable final steps.

The existing unregularized batch tests now compare feasibility and objective values rather than a specific exact-OT plan, since exact plans need not be unique. They also use a convergence-appropriate proximal coefficient for the corrected recurrence; regularized cases retain their elementwise plan comparisons and prior coefficient.

How has this been tested

  • Regression proof against the released implementation: two-step NumPy log cross-ratio was approximately 0 instead of expected 6.
  • python -m pytest -q test_solve_batch_issue842.py --no-cov -o addopts=: 75 passed (complete test/batch/test_solve_batch.py, NumPy and Torch backends available).
  • pre-commit run --files ot/batch/_utils.py test/batch/test_solve_batch.py RELEASES.md: passed (Ruff lint, Ruff preview lint, Ruff format, codespell).
  • git diff --check: passed.

The tests ran on Python 3.13 using the official POT 0.9.7 Windows wheel for compiled extensions with the changed pure-Python batch module overlaid. A local editable source build was unavailable because this Windows host does not have MSVC; repository CI will exercise the normal source build.

PR checklist

  • I have read the CONTRIBUTING document.
  • The documentation is up-to-date with the changes I made.
  • All targeted tests passed, and the fix is covered with new regression tests.
  • I have added the issue fix to RELEASES.md.

@rflamary

Copy link
Copy Markdown
Collaborator

Hello @ANONYMOUSZED-beep , I'm closing this PR since it was fixed in #844 and it now has some conflicts.

@rflamary rflamary closed this Aug 28, 2026
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.

[Bug] Incorrect sign of the previous transport plan in proximal_bregman_log_plan_batch

2 participants