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)

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