[MRG] Gracefully handle cost = 0 in generic_conditional_gradient

[kachayev]

Types of changes

relative_delta_cost_G in generic_conditional_gradient is only computed when the cost != 0.

Motivation and context / Related issue

As the contract for cost evaluation in generic_conditional_gradient is rather generic, it could be the case it's set to 0. In this case computing relative_delta_cost_G fails because of zero division. As we only need the value of relative_delta_cost_G for early stopping condition, it doesn't hurt to skip the computation when cost is 0.

The code to reproduce the issue:

import numpy as np
import ot
import ot.datasets

n_samples = 50
mu_s = np.array([0, 0])
cov_s = np.array([[1, 0], [0, 1]])
xs = ot.datasets.make_2D_samples_gauss(n_samples, mu_s, cov_s, random_state=4)
xt = xs[::-1].copy()
p = ot.unif(n_samples)
q = ot.unif(n_samples)
G0 = p[:, None] * q[None, :]
C1 = ot.dist(xs, xs)
C2 = ot.dist(xt, xt)
C1 /= C1.max()
C2 /= C2.max()
ot.gromov.gromov_wasserstein(C1, C2, p, q, 'square_loss', G0=G0, verbose=True)

How has this been tested (if it applies)

This actually happens in test_gromov_dtype_device test case. The test case is not modified to ensure we don't have warnings when computing ot.gromov.gromov_wasserstein.

PR checklist

• I have read the CONTRIBUTING document.
• The documentation is up-to-date with the changes I made (check build artifacts).
• All tests passed, and additional code has been covered with new tests.
• I have added the PR and Issue fix to the RELEASES.md file.

[codecov]

Codecov Report

Merging #505 (441a3a7) into master (7856700) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #505   +/-   ##
=======================================
  Coverage   96.25%   96.25%           
=======================================
  Files          67       67           
  Lines       14040    14043    +3     
=======================================
+ Hits        13514    13517    +3     
  Misses        526      526           

[cedricvincentcuaz]

Hello @kachayev,

Good catch ! However I am not sure that setting 'relative-delta_cost_G = np.nan' if 'cost_G = 0' is the most appropriate, as it would imply '(relative_delta_cost_G < stopThr) == False'. Then let the condition 'abs_delta_cost_G < stopThr2' handles the job, probably given 1 more round of the CG loop. On one hand, in several cases the cost is non-negative so we might want to force the loop to stop. On the other hand, this cg solver is meant to be as generic as possible. So maybe the best approach would be to add an optional parameter that allows to control this behaviour. Wdyt @kachayev @rflamary ?

[kachayev]

Hi @cedricvincentcuaz!

I basically set it to 'man' to have the same behavior it has right now, just without performing division.

We could add additional flag to force exit from the loop when 0 is reached. Though for continuous space feels like an edge case. Should it be a parameter to stop when the cost is lower than given 'eps' rather than "exactly 0"? 🤔

[cedricvincentcuaz]

Hello @kachayev,
Sorry for the late answer. In the end, these indeed seem to be really specific edge cases, I am not so sure that it is worth it to add dedicated tests to gain one iteration when these occur.
I suggest to keep the previous behaviour as your code currently does. So we could merge after tests are completed.

Overall, I think that it would be nice to go a normalisation of stopping criterions for various solvers (at least two consistent types) in POT. So our current interrogations could be handled in this context.

Best, Cédric.