add stochastic methods #52

kilianFatras · 2018-06-06T23:17:20Z

SAG for discrete measures and ASGD for semi continuous measure.

rflamary · 2018-06-11T09:56:42Z

Hello @kilianFatras and thank you for the job. We needed the stochastic implementation and you provided a good implementation there.

But there are still a few things that need to be done before merging

The PR do not build because of pep8 problems and failing tests.
The submodule stochastic is not imported in module ot which is why it does not work in the test. You need to add from . import stochastic to the ot/__init__.py file in order to import it while doing import ot.
The order of the parameters in all your function should follow closely the order in other functions such as ot.emd and ot.sinkhorn so that you can switch from one to the other easily (at least a,b M , and reg (which you call epsilon and should be renamed also) followed by optimization parameters).
n_source and n_target are not useful since those dimensions can be inferred from a,b or M. You also should provide default values for n_iter (renamed to numItermax to fit ot.emd and ot.sinkhorn functions) and lr.
In the test i would like you to also test that your solver return the same solution as the ot.sinkkorn function. This is an important test since both solver address the same optimization problem with different algorithms.
please use POT functions for providing uniform distributions and computing distance matrices M (in the doc, the test and the examples). uniform distributions of dimensionality n are obtained easily by ot.unif, and M can be computed very easily with ot.dist function.
We try to provide in the documentation of the function the optimization problem that is solved (see ot.emd or ot.sinkhorn). So plesa express the dual problem solved by the saga and asgd function. The c transform should also appear in the documentation since it can be used in numerous occasions. Also try to provide the euaton or algorithm number of the reference paper use in the documentation of all the functions. It helps the user/reader to better follow what is done.

Note that all the tests are performed again after you do a commit push.

Again thank you for the job, those are only remarks aiming at having a better integration of your algorithm into POT.

Rémi

rflamary

Thank you @kilianFatras,

the code looks great, I left a few comments

rflamary · 2018-06-18T06:33:15Z

ot/stochastic.py

+    return u
+
+
+def transportation_matrix_entropic(a, b, M, reg, method, numItermax=10000,


rename the function to solve_entropic

or solve_semi_dial_entropic

rflamary · 2018-06-18T06:33:57Z

ot/stochastic.py

+    for i in range(n_source):
+        r = M[i, :] - v
+        exp_v = np.exp(-r / reg) * b
+        u[i] = - reg * np.log(np.sum(exp_v))


should we use a stabilized version of logsumexp?
https://en.wikipedia.org/wiki/LogSumExp

so that the algorithm do not explode

rflamary · 2018-06-18T06:38:36Z

test/test_stochastic.py

+    np.testing.assert_allclose(
+        zero, (G_asgd - G_sinkhorn).sum(1), atol=1e-03)  # cf convergence asgd
+    np.testing.assert_allclose(
+        zero, (G_asgd - G_sinkhorn).sum(0), atol=1e-03)  # cf convergence asgd


also test the whole matrix and not onmly the marginals with np.testing.assert_allclose(G_asgd nG_sinkhorn, atol=1e-03)

rflamary · 2018-06-18T06:42:32Z

ot/stochastic.py

+    opt_u = c_transform_entropic(b, M, reg, opt_v)
+    pi = (np.exp((opt_u[:, None] + opt_v[None, :] - M[:, :]) / reg) *
+          a[:, None] * b[None, :])
+    return pi


Please add a log parameter to the function that return a dictionary with u and v (probably better name alpha and beta see sinkhorn/emd wher emd return also the dual)

rflamary

Hello @kilianFatras,

The code is far better now and the function names are much clearer.

The example file do not run due to a small bug discussed in the comments. Also I have seen some problems with documentation compilation due to small typos and missing lines.

The dual SGD is a bit slow but i guess it is difficult to compete with SAG and ASGD.

Again thank you for your work, we will merge shortly now.

rflamary · 2018-06-25T14:40:27Z

examples/plot_stochastic.py

+
+method = "ASGD"
+asgd_pi, log = ot.stochastic.solve_semi_dual_entropic(a, b, M, reg, method,
+                                                      numItermax, log)


bug because log is the wrong positional argument. Replace log by log_asgd is the left hand side and log by log=log in the right hand side.

rflamary · 2018-06-25T14:41:42Z

examples/plot_stochastic.py

+
+import matplotlib.pylab as pl
+import numpy as np
+import ot


also import ot.plot because it is not imported by default

rflamary · 2018-06-25T14:44:16Z

ot/stochastic.py

+    Compute the coordinate gradient update for regularized discrete
+        distributions for (i, :)
+
+    The function computes the gradient of the semi dual problem:


add line before math operator for proper math generation in documentation

rflamary · 2018-06-25T14:44:31Z

ot/stochastic.py

+        optimal transport max problem
+
+    The function solves the following optimization problem:
+    .. math::


rflamary · 2018-06-25T14:44:38Z

ot/stochastic.py

+        optimal transport max problem
+
+    The function solves the following optimization problem:
+    .. math::


rflamary · 2018-06-25T14:44:51Z

ot/stochastic.py

+        measures optimal transport max problem
+
+    The function solves the following optimization problem:
+    .. math::


rflamary · 2018-06-25T14:45:30Z

ot/stochastic.py

+    Computes the partial gradient of F_\W_varepsilon
+
+    Compute the partial gradient of the dual problem:
+    ..Math:


same here, use math instead of Math

rflamary · 2018-06-25T14:46:35Z

ot/stochastic.py

+    Computes the partial gradient of F_\W_varepsilon
+
+    Compute the partial gradient of the dual problem:
+    ..Math:


same here, use math instead of Math

rflamary · 2018-06-25T14:46:50Z

ot/stochastic.py

+        optimal transport dual problem
+
+    The function solves the following optimization problem:
+    .. math::


rflamary · 2018-06-25T14:47:00Z

ot/stochastic.py

+        optimal transport dual problem
+
+    The function solves the following optimization problem:
+    .. math::


same here, use math instead of Math

kilianFatras · 2018-06-25T21:52:22Z

I think that SGD's performance also depends on the hyperparameters. It is really hard to find the good batch size or the learning rate. I tried many but as it is the dual problem, it is not as easy as for the semi dual problem. In [Genevay & al.], they pointed out that for the semi dual problem, you can have an upper bound of the Lipschitz constant which is not the case for the dual problem. I will keep working on this as it can increase the convergence speed.

Kilian Fatras added 2 commits June 15, 2018 18:53

add problems solved in doc

c8eda44

add problems solved in doc

e2d06ef

kilianFatras force-pushed the stochastic_OT branch from ef14ddb to e2d06ef Compare June 16, 2018 02:05

Kilian Fatras added 2 commits June 15, 2018 19:15

PEP8

18f9242

PEP8

3617b42

kilianFatras force-pushed the stochastic_OT branch from 4008808 to 3617b42 Compare June 16, 2018 02:18

pep8

055417e

kilianFatras force-pushed the stochastic_OT branch from 14e4aaa to 055417e Compare June 16, 2018 02:26

rflamary reviewed Jun 18, 2018

View reviewed changes

add sgd

74cfe5a

kilianFatras force-pushed the stochastic_OT branch from dd3398b to 74cfe5a Compare June 19, 2018 00:56

pep8

e068b58

kilianFatras force-pushed the stochastic_OT branch from e77c487 to e068b58 Compare June 19, 2018 01:05

change grad function names

52134e9

kilianFatras force-pushed the stochastic_OT branch from f1fa080 to 52134e9 Compare June 19, 2018 18:31

remove if in test and cleaned code

7073e41

kilianFatras force-pushed the stochastic_OT branch from 49a50b6 to 7073e41 Compare June 19, 2018 18:52

gave better step size ASGD & SAG

6777ffd

kilianFatras force-pushed the stochastic_OT branch from 6837b9c to 6777ffd Compare June 22, 2018 00:18

fixed bug

af5f726

kilianFatras force-pushed the stochastic_OT branch from 4aa054f to af5f726 Compare June 22, 2018 00:41

pep8

e8cf3cc

kilianFatras force-pushed the stochastic_OT branch from 03d7e25 to e8cf3cc Compare June 22, 2018 00:52

rflamary reviewed Jun 25, 2018

View reviewed changes

fix math operator and log bugs

9fecd51

kilianFatras force-pushed the stochastic_OT branch from 29f025a to 9fecd51 Compare June 25, 2018 18:03

add stochastic to docs

968ad58

kilianFatras force-pushed the stochastic_OT branch from 011f5e1 to 968ad58 Compare June 26, 2018 18:00

fix stochastic to doc

208ff46

kilianFatras force-pushed the stochastic_OT branch from 64d504b to 208ff46 Compare June 26, 2018 18:10

Merge branch 'master' into stochastic_OT

b4bc861

rflamary merged commit 39cbcd3 into PythonOT:master Jun 27, 2018

		return u


		def transportation_matrix_entropic(a, b, M, reg, method, numItermax=10000,

add stochastic methods #52

add stochastic methods #52

Uh oh!

Conversation

kilianFatras commented Jun 6, 2018

Uh oh!

rflamary commented Jun 11, 2018

Uh oh!

rflamary left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rflamary left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kilianFatras commented Jun 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kilianFatras commented Jun 25, 2018 •

edited

Loading