-
Notifications
You must be signed in to change notification settings - Fork 528
[WIP] OT barycenters for generic transport costs #715
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.
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #715 +/- ##
==========================================
+ Coverage 97.10% 97.14% +0.04%
==========================================
Files 101 101
Lines 20493 20910 +417
==========================================
+ Hits 19900 20314 +414
- Misses 593 596 +3 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @eloitanguy this is very nice work as usual. I have a couple comments below
After discussion with @rflamary we decided to expand the PR with additional features, namely two barycenter solvers:
|
After some updates to the paper behind these algorithms (the paper update is soon to come), I implemented another barycenter solver which corresponds exactly to the method studied theoretically in the paper (iterates of G). I updated this PR with the additional algorithm ( On my end, this contribution is ready for review :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this very nice PR @eloitanguy ! :)
Follows some comments on the generic barycentre solver for now. I'll revise the GMM parts next.
examples/barycenters/plot_free_support_barycenter_generic_cost.py
Outdated
Show resolved
Hide resolved
n = 136 # number of points of the of the barycentre | ||
d = 2 # dimensions of the original measure | ||
K = 4 # number of measures to barycentre | ||
m = 50 # number of points of the measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we take different number of points ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the number of points can be chosen as desired (no need for equality anywhere). What do you mean by your comment? Are you asking for another set of parameters for the illustration?
Hi @cedricvincentcuaz , thanks a lot for your thorough code review! I went through your comments and applied the modifications for most of them. I left the conversations open only for your comments which warrant further discussion or verification. |
Types of changes
free_support
that accepts any cost function (implements this paper)ot.gmm
for fast computation of GMM barycentersREADME.md
For the (theoretical) fixed-point method, use
method='true_fixed_point'
inot.lp.free_support_barycenter_generic_costs
and for the barycentric heuristic, usemethod='L2_barycentric_proj'
. The latter is the default, given the computational advantages and the desirable property of keeping a fixed support size.Motivation and context / Related issue
How has this been tested (if it applies)
test/test_ot.py
andtest/test_gmm.py
PR checklist