Skip to content

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

Description

@WenzhouXia

Describe the bug

The sign of the previous transport plan appears to be incorrect in
proximal_bregman_log_plan_batch.

The current implementation uses:

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

The same expression is also used in the grad == "last_step" branch.

However, the proximal subproblem documented immediately above the
implementation is

$$ \min_{T \in \Pi(a,b)} \langle C-\beta\log T^{(k)},T\rangle +(\mathrm{reg}+\beta)\sum_{ij}T_{ij}\log T_{ij}, $$

where beta = inner_reg.

Therefore, the corresponding log-kernel should be

$$ K^{(k)}= -\frac{C-\beta\log T^{(k)}}{\mathrm{reg}+\beta}, $$

which corresponds to:

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

For the unregularized case (reg = 0), this becomes

K_proj = log_T - C / inner_reg

which agrees with Algorithm 1 of Xie et al. (2020) and the authors'
reference implementation:

Q = exp(-C / beta) * P

The current implementation instead gives

K_proj = -log_T - C / inner_reg

which reverses the contribution of the previous transport plan.

References

Suggested fix

Replace both occurrences of

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

with

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

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions