[MRG] Initialization for Low Rank Sinkhorn solver #588
Conversation
rflamary
changed the title
There was a problem hiding this comment.
Hello @laudavid,
thanks a lot for your PR.
Here are a few remarks to fix and also questions to clarify some points.
Best,
Cédric
ot/lowrank.py
Outdated
| R = nx.abs(nx.randn(nt, rank, type_as=X_s)) + 1 | ||
| R = (R.T * (b / nx.sum(R, axis=1))).T | ||
|
|
||
| if init == "trivial": |
There was a problem hiding this comment.
probably the most "trivial" init to do would be to set g uniform and deduce Q (resp. R) following equations in lines 84-85 (resp. 88-89). Would such initialization be of interest ? is trivial a proper name for this other deterministic init ?
There was a problem hiding this comment.
This initialization strategy was called "trivial" by the researchers.
I don't really know how to name it otherwise.
I can also remove this option for init, since there is also a "random" and "kmeans" option.
There was a problem hiding this comment.
Maybe 'deterministic', to contrast to 'random' which are the two initializations which are data independent.
There was a problem hiding this comment.
Thank you for your suggestion. I'll change it to "deterministic".
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #588 +/- ##
==========================================
+ Coverage 96.73% 96.74% +0.01%
==========================================
Files 77 77
Lines 15820 15911 +91
==========================================
+ Hits 15303 15393 +90
- Misses 517 518 +1 |
ot/lowrank.py
Outdated
| R = nx.abs(nx.randn(nt, rank, type_as=X_s)) + 1 | ||
| R = (R.T * (b / nx.sum(R, axis=1))).T | ||
|
|
||
| if init == "trivial": |
There was a problem hiding this comment.
Maybe 'deterministic', to contrast to 'random' which are the two initializations which are data independent.
rflamary
changed the title
* new file for lr sinkhorn * lr sinkhorn, solve_sample, OTResultLazy * add test functions + small modif lr_sin/solve_sample * add import to __init__ * modify low rank, remove solve_sample,OTResultLazy * new file for lr sinkhorn * lr sinkhorn, solve_sample, OTResultLazy * add test functions + small modif lr_sin/solve_sample * add import to __init__ * remove test solve_sample * add value, value_linear, lazy_plan * add comments to lr algorithm * modify test functions + add comments to lowrank * modify __init__ with lowrank * debug lowrank + test * debug test function low_rank * error test * final debug of lowrank + add new test functions * Debug tests + add lowrank to solve_sample * fix torch backend for lowrank * fix jax backend and skip tf * fix pep 8 tests * add lowrank init + test functions * Add init strategies in lowrank + example (#588) * modified lowrank * changes from code review * fix error test pep8 * fix linux-minimal-deps + code review * Implementation of LR GW + add method in __init__ * add LR gw paper in README.md * add tests for low rank GW * add examples for Low Rank GW * fix __init__ * change atol of lr backends * fix pep8 errors * modif for code review
Hello Rémi,
Here is a PR to add initialization strategies to the low rank sinkhorn solver (for Q, R, g).
I added a "random", "trivial" and "kmeans" strategy and created a new function
_init_lr_sinkhornto implement them.