-
Notifications
You must be signed in to change notification settings - Fork 537
[MRG] Restructure and Augment Partial Gromov-Wasserstein solvers #663
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
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 |
rflamary
left a comment
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.pytoot/gromov/_partial.pywhile adding a depreciation notes and warnings withinot/partial.pyto 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_matrixreturning 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_matrixandsemirelaxed_init_matrix.Then adapt solvers to mimic other GW related solvers in order to ease future integration of new solvers (e.g pFGW ones):
pandqoptional and set by default to uniform ones.loss_funparameter within ['square_loss', 'kl_loss'] -> now solvers supportkl_losstoo.symmetryparameter within [None, True, False] -> now solvers support asymmetric structure matrices.gwloss,gwggrad), therefore functionsgwgrad_partialandgwloss_partialinot/partial.pywill also be depreciated (kept for now with a de.partial_cginot/optim.py.ot/optim.py.solve_partial_gromov_linesearchfor 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_cgsolver. 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