Skip to content

Conversation

@little-nem
Copy link
Contributor

The alpha trade-off parameter of the Fused Gromov Wasserstein implementation was not used correctly: what was computed was

(arg)min_\gamma <\gamma, M>_F + \alpha \sum L(C1_{i,k},C2_{j,l}) T_{i,j} T_{k,l}

instead of the convex combination

(arg)min_\gamma (1 - \alpha) <\gamma, M>_F + \alpha \sum L(C1_{i,k},C2_{j,l}) T_{i,j} T_{k,l}

Even though the two are mathematically equivalent up to a rescaling of alpha, it was not consistent with the documentation. I changed the implementation so optim.cg is called with cg(..., (1-alpha)*M, alpha, ...) instead of cg(..., M, alpha, ...) which should be enough to fix this issue.

@little-nem little-nem changed the title [MRG] Fix Fused Gromov Wasserstein implementation [WIP] Fix Fused Gromov Wasserstein implementation Mar 10, 2020
@little-nem little-nem changed the title [WIP] Fix Fused Gromov Wasserstein implementation [MRG] Fix Fused Gromov Wasserstein implementation Mar 10, 2020
@rflamary
Copy link
Collaborator

This seems OK to me. Thank you for the PR.

Will merge in a few days.

@rflamary rflamary merged commit fa06bb3 into PythonOT:master Mar 19, 2020
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.

2 participants