-
Notifications
You must be signed in to change notification settings - Fork 502
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
[MRG] Restructure and Augment Partial Gromov-Wasserstein solvers #663
[MRG] Restructure and Augment Partial Gromov-Wasserstein solvers #663
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #663 +/- ##
==========================================
+ Coverage 97.03% 97.06% +0.02%
==========================================
Files 96 98 +2
Lines 19255 19638 +383
==========================================
+ Hits 18685 19061 +376
- Misses 570 577 +7 |
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.
A few remaining comment mainly about documentation.
After that I think we can merge. This is an impressive amount of work.
Types of changes
create duplicates of partial GW related functions from
ot/partial.py
toot/gromov/_partial.py
while adding a depreciation notes and warnings withinot/partial.py
to maintain the API for now. This includes the following functions:partial_gromov_wasserstein
,partial_gromov_wasserstein2
,entropic_partial_gromov_wasserstein
,entropic_partial_gromov_wasserstein2
.Add generic (hidden function)
_transform_matrix
returning transformed structure matrices for GW inot/gromov/_utils.py
(used to compute only once these transformations in pGW even though there is not constant terms a priori as with the GW problem) and use it to factor code ininit_matrix
andsemirelaxed_init_matrix
.Then adapt solvers to mimic other GW related solvers in order to ease future integration of new solvers (e.g pFGW ones):
p
andq
optional and set by default to uniform ones.loss_fun
parameter within ['square_loss', 'kl_loss'] -> now solvers supportkl_loss
too.symmetry
parameter within [None, True, False] -> now solvers support asymmetric structure matrices.gwloss
,gwggrad
), therefore functionsgwgrad_partial
andgwloss_partial
inot/partial.py
will also be depreciated (kept for now with a de.partial_cg
inot/optim.py
.ot/optim.py
.solve_partial_gromov_linesearch
for the exact line-search of pGW. Note that the latter requires the computation of the gradient of the regularizer atG
(like other cg functions) but also at the cg directionGc
. This allows to deduce the next step gradient as a convex combinations of these previous gradients (should reduce the computation time of about 33 %), within thegeneric_cg
solver. This trick will be implemented for all (F)GW based regularizer in a next PR.Motivation and context / Related issue
How has this been tested (if it applies)
PR checklist