unrespected bounds

[AdrienCorenflos]

Hi,

There is a (subtle) bug in EMD_1d: the indices i and j are allowed to reach n and m, therefore these lines have unkown behaviour (the resulting values w_i and w_j are dereferenced from the next memory slot in the machine, therefore it could be any value or even fail depending on compiler).

An easy fix would be to change the while condition to a while True and add break condiditions within the if-else branches.

POT/ot/lp/emd_wrap.pyx

Line 191 in 0a1e8cd

w_i = u_weights[i]

POT/ot/lp/emd_wrap.pyx

Line 199 in 0a1e8cd

w_j = v_weights[j]

I'll send a pull request if you are happy with this alternative

[rtavenar]

Hi @AdrienCorenflos

Good catch once again! Another easy fix would be to set w_i and w_j at the beginning of each iteration of the while loop (instead of at the end of the iteration).
You could implement the one you prefer, I guess, and a PR would be very welcome :)

[AdrienCorenflos]

I'd rather not change the logic too much to be honest, cython allows for breaks in loops anyway.

(also the only reason why I spotted it is because I use your code as a benchmark, I am implementing something very similar that directly computes Gamma x U :))

[AdrienCorenflos]

Btw, there are a few things that could be improved in this cython code, mostly compatibility-wise. You should use fuse types to be able to use float32 or float64 types seemlessly for example

[rtavenar]

I am a newbie in Cython, so any improvement to the code would be welcome, feel free to do that in the PR as well.

[AdrienCorenflos]

errr I should have run it locally, tests are failing

[AdrienCorenflos]

I am a newbie in Cython, so any improvement to the code would be welcome, feel free to do that in the PR as well.

That's gonna be more work, I'll fix the bug and open a new issue for later on

[AdrienCorenflos]

tests are passing