-
Notifications
You must be signed in to change notification settings - Fork 528
[FEAT] Entropic gw/fgw/srgw/srfgw solvers #455
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
[FEAT] Entropic gw/fgw/srgw/srfgw solvers #455
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #455 +/- ##
==========================================
+ Coverage 94.92% 95.15% +0.22%
==========================================
Files 31 31
Lines 6879 7137 +258
==========================================
+ Hits 6530 6791 +261
+ Misses 349 346 -3 |
…tcuaz/POT into entropic_gw_solvers
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.
very nice PR, next time do a smaller one but keep on with the quality code ;).
Small comments about usability below
Thanks for the review Rémi :) One thing I just realised. In entropic gw and fgw, there is no test on marginals if an initial transport plan G0 is given (different than None), should we add them ? |
…o entropic_gw_solvers
Types of changes
changes to file ot/gromov/_bregman.py:
new features in
entropic_gromov_wasserstein
(2):i)
solver
: Set by default tosolver='PGD'
to not change the Projected Gradient Descent solver already implemented in the API. Addsolver='PPA'
to use the Proximal Point Algorithm from [50].ii)
warmstart
: Either to perform warmstart of dual potentials in the successive Sinkhorn projections.iii)
kwargs
: to pass parameters toot.sinkhorn
solver e.g to control its maximum number of iterations.new features in
entropic_gromov_barycenters
:i)
warmstartT
: Either to perform warmstart of transport plans in the successive gromov-wasserstein transport problems.ii)
kwargs
: to pass parameters toot.entropic_gromov_wasserstein
e.g usingsolver='PGD'
orsolver='PPA'
.new functions:
entropic_fused_gromov_wasserstein
,entropic_fused_gromov_wasserstein2
,entropic_fused_gromov_barycenters
that extend existing functions for entropic GW to graphs with node features via FGW.changes to file ot/gromov/_gw.py:
gromov_barycenters
andfgw_barycenters
:i)
warmstartT
: Either to perform warmstart of transport plans in the successive (fused) gromov-wasserstein transport problems.ii) Fix bug in the update over structures when using
loss_fun='kl_loss'
update_structure_matrix
which was equivalent toot.gromov._utils.update_square_loss
changes to file ot/gromov/_semirelaxed.py:
entropic_semirelaxed_gromov_wasserstein
,entropic_semirelaxed_gromov_wasserstein2
,entropic_semirelaxed_fused_gromov_wasserstein
,entropic_semirelaxed_fused_gromov_wasserstein2
: mirror descent algorithms following the KL geometry to solve semi-relaxed (Fused) Gromov-Wasserstein problems.changes to file ot/gromov/_utils.py:
ot.gromov._gw.update_feature_matrix
toot.gromov._utils.update_feature_matrix
as now use in both_gw.py
and_bregman.py
Nb : The marginals have been made optional for all solvers in these files.
Motivation and context / Related issue
Keep up with the (F)GW literature
How has this been tested (if it applies)
changes to file ot/gromov/_bregman.py:
new tests:
test_entropic_proximal_gromov
,test_entropic_proximal_fgw
,test_entropic_fgw
,test_asymmetric_entropic_fgw
,test_entropic_fgw_dtype_device
,test_entropic_fgw_barycenter
modified tests:
test_entropic_gromov_dtype_device
,test_entropic_fgw_dtype_device
,test_gromov_entropic_barycenter
changes to file ot/gromov/_gw.py:
modified tests:
test_gromov_barycenter
,test_fgw_barycenter
changes to file ot/gromov/_semirelaxed.py:
new tests:
test_entropic_semirelaxed_gromov
,test_entropic_semirelaxed_gromov_dtype_device
,test_entropic_semirelaxed_fgw
,test_entropic_semirelaxed_fgw_dtype_device
test_not_implemented_solver
for entropic solvers andloss_fun=kl_loss
for semi-relaxed solvers.PR checklist