Skip to content

[ENH] Stabilized and epsilon scaled sinkhorn solvers #84

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 37 commits into from
Apr 17, 2025
Merged

[ENH] Stabilized and epsilon scaled sinkhorn solvers #84

merged 37 commits into from
Apr 17, 2025

Conversation

@pbarbarant
Copy link
Collaborator

@pbarbarant pbarbarant commented Mar 12, 2025 •
edited
Loading

This PR implements stabilized and epsilon scaled sinkhorn solvers (Schmitzer, B. 2016), supporting sparse cost matrices.

Schmitzer, B. (2016). Stabilized Sparse Scaling Algorithms for
        Entropy Regularized Transport Problems.
        arXiv preprint arXiv:1610.06519.

Pierre-Louis Barbarant and others added 20 commits March 12, 2025 17:01
Update pre-commit config to python 3.12
1dfd807
Downgrade black language version to python 3.11 in pre-commit config
64b9dcc
Add stabilized Sinkhorn algorithms with sparse support and epsilon sc…
3228ee4 
…aling
Add unit tests for Sinkhorn solvers in test_utils.py
d603005
Replace solver_sinkhorn with solver_sinkhorn_log in FUGWSolver and ad…
658bb05 
…just logging format
Replace solver_sinkhorn_sparse with solver_sinkhorn_log_sparse in FUG…
7eee839 
…WSparseSolver
Apply final potential transformation before computing the transport plan
30220de
Add testing of transport plan equality
271e333
Update Sinkhorn solver tests to include transport plan checks and adj…
dea3159 
…ust evaluation frequency
Refactor Sinkhorn solver to replace pi_diff with err for convergence …
7a83ccc 
…checks and update logging messages
Remove debug print statement for error in Sinkhorn EPS scaling solver
4f4528f
Enhance progress bar function to support null context when verbosity …
6bdf7a7 
…is disabled
Refactor progress tracking in Sinkhorn solvers to include verbosity o…
9fac2a2 
…ption
Import ot only if needed in init_plan_dense
68e4611
Remove numInnerItermax parameter from solver_sinkhorn_eps_scaling fun…
31f34e7 
…ction and adjust train_params unpacking
@pbarbarant
Updated stabilization threshold
0858858
Refactor Sinkhorn solver functions to improve stability and transport…
3ae891a 
… plan computation
Adjust stabilization threshold and implement sparse transport plan co…
ecc2d16 
…mputation
Adjust tolerance levels in Sinkhorn solver tests for improved accuracy
1694553
Improve docstring formatting for progress bar function
6dc8d14
bthirion
bthirion reviewed Mar 26, 2025
View reviewed changes
Copy link
Collaborator

@bthirion bthirion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious. This solver is not in POT ?

There's a cost in maintaining our own solvers...

bthirion
bthirion reviewed Mar 26, 2025
View reviewed changes
@pbarbarant pbarbarant marked this pull request as ready for review March 27, 2025 13:37
@pbarbarant
Copy link
Collaborator Author

pbarbarant commented Mar 27, 2025

Just curious. This solver is not in POT ?

There's a cost in maintaining our own solvers...

Dense solvers are similar to POT, but the sparse implementations are unique to FUGW.

@pbarbarant pbarbarant changed the title [WIP] Stabilized and epsilon scaled sinkhorn solvers [ENH] Stabilized and epsilon scaled sinkhorn solvers Mar 27, 2025
@pbarbarant pbarbarant added the enhancement New feature or request label Mar 27, 2025
bthirion
bthirion approved these changes Mar 27, 2025
View reviewed changes
Copy link
Collaborator

@bthirion bthirion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't spot any issue.
At the end of the day, how many solvers do we support ? Do we have a clear use case for all of them ?

bthirion
bthirion reviewed Apr 2, 2025
View reviewed changes
Copy link
Collaborator

@bthirion bthirion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the code could be organized a bit better ?

@pbarbarant
Copy link
Collaborator Author

pbarbarant commented Apr 2, 2025

I removed the dense solvers which are already implemented in POT, which means less code to maintain.

@pbarbarant
Copy link
Collaborator Author

pbarbarant commented Apr 2, 2025

Maybe the code could be organized a bit better ?

I removed the nested functions.

@pbarbarant pbarbarant merged commit be13850 into main Apr 17, 2025
7 checks passed
@pbarbarant pbarbarant deleted the feat/stabilized-sinkhorn branch April 17, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bthirion bthirion bthirion approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pbarbarant @bthirion