Skip to content
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

added optimal transport example #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

sbarratt
Copy link
Collaborator

@olivierteboul can you take a look at this?

@sbarratt
Copy link
Collaborator Author

@akshayka

@bamos
Copy link
Collaborator

bamos commented Apr 14, 2020

I think this is the same as Eq (3) of https://arxiv.org/abs/1802.08665, right?

image

If so, it would be cool to mention the connection to that paper, and to https://arxiv.org/abs/1106.1925, since they differentiate through that convex optimization problem by unrolling the sinkhorn iterates. (Which is exact if the sinkhorn iterates converge)

@sbarratt
Copy link
Collaborator Author

sbarratt commented Apr 14, 2020 via email

@akshayka
Copy link
Member

akshayka commented Apr 21, 2020

Sorry for the late response ...!

Looks mostly good! Just a few comments.

  1. I was confused by the wording in the first sentence, since the support of a probability distribution is typically a set, not a vector.
  2. The LaTeX isn't rendering correctly. I'm looking here: https://github.com/cvxgrp/cvxpylayers/blob/970163a34770bca0daac7cb727627e45fa03c169/examples/torch/optimal_transport.ipynb
  3. Maybe include axis labels on the plot, to explain what's being visualized?

@akshayka
Copy link
Member

LGTM modulo the comments I made previously.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants