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

Enable to set parameters for inner Sinkhorn loop in Gromov-Wasserstein #160

Closed
MUCDK opened this issue Oct 20, 2022 · 4 comments
Closed

Comments

@MUCDK
Copy link
Contributor

MUCDK commented Oct 20, 2022

Currently, it is not possible to set parameters like max_iterations and min_iterations of the inner Sinkhorn loop when calling WassersteinSolver or GromovWasserstein because of the same argument names.

It would be great to rename the outer loop parameters to have more control about the algorithm.

@marcocuturi
Copy link
Contributor

marcocuturi commented Oct 20, 2022

Thanks Dominik for the great comment.

I think this is possible using the most recent API, as illustrated in this colab:

https://colab.research.google.com/drive/1YWKKjSWnpbhpeMae2tlkHKLLZXP97Cij?usp=sharing

You need to instantiate manually a QuadraticProblem, use a GromovWasserstein solver on it, and specify in GromovWasserstein the linear_ot_solver (this is common for all WassersteinSolver.
Namely, the linear solver inside the GW solver can be provided explicitly, with all parameters you might want, which is what happens in that call in the colab

GWsolver= gw.GromovWasserstein(
    linear_ot_solver=sinkhorn.Sinkhorn(
      min_iterations=1, 
      max_iterations=30,
      threshold=1e-1),
    min_iterations=1,
    max_iterations=7,
    epsilon=100.0,
    )

and then simply make the call

quad_p = qp.QuadraticProblem(geom_xx, geom_yy, scale_cost=True)
out = GWsolver(quad_p)

In the colab you can see how the .errors field of the output of GW shows the right size, showing how max_iterations are properly handled.

Using such classes, it is easy to have access to detailed granularity on the components that make up an OT problem. I think it's also a lot more viable than overloading the signature of gromov_wasserstein with sinkhorn_kwargs or sinkhorn_lr__kwargs etc... This is also why @michalk8 wants to get rid of make functions #131

@MUCDK
Copy link
Contributor Author

MUCDK commented Oct 22, 2022

Thanks Marco!
I like the idea with sinkhorn_kwargs but the current approach is also nice!
Just want to make sure you are not about to refactor this?

@marcocuturi
Copy link
Contributor

yes, the example I provided is clearly te way Michal and I want to go. We'll still maintain the sinkhorn and gromov_wasserstein wrappers for a little while, but I think we'll deprecate them at some point!

@MUCDK
Copy link
Contributor Author

MUCDK commented Oct 23, 2022

Amazing, thank you!

@MUCDK MUCDK closed this as completed Oct 23, 2022
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

No branches or pull requests

2 participants