[MRG] add screenkhorn: screening Sinkhorn algorithm

[mzalaya]

No description provided.

[rflamary]

Hello @mzalaya,

Thank you for your PR. I think screenkhorn has its place in POT. I have several comments that should be taken care of before marge. Note that i don't want to add new dependencies so you should code your function to work with numpy if bottleneck is not installed (and use bottleneck if installed).

You will also need to respect pep8. you can do an automatic autopep8 with make aautopep8 in the POT main folder.

Finally you need to check that all tests are passing and you need to write tests to check that your function runs without bugs and return a meaningful solution in test/test_bregman.py

Welcome to the POT contributors

[rflamary]

Hello @mzalaya,

I see you have implemented a lot of our comments and the code is shaping up very well.

There is still the binary indexing that needs to be looked at and I would also like you to add an example of you solver in the example folder of the repository. The examples are afterwards included semi automatically in the documentation with the plots and everything and it will definitely make your new contribution more visible. Also we need some tests on the new function that i don't see running in travis yet.

I triggered the build for your last commit to see if the tests pass. I don't know why it did not trigger itself (spoiler alert it failed because of pep8 violations).

[rflamary]

Hello @mzalaya,

We are nearly there. What is still missing before merge:

• Test should pass: use autopep8 on the example file also
• Add a test that call your function ,and check that the output makes sens.
• you should use r"""doc""" for using the latex backslash in the documentatiion

[mzalaya]

@rflamary I think screenkhorn is ready to be merged in POT : )

[rflamary]

The function should not fail when bottleneck is not installed and it will because it uses bottleneck afterwards. As already said we don't want to add dependencies.

i would do something like

try:
    import bottleneck
except ImportError:
    warning("Bottleneck module is not installed. Install it from https://pypi.org/project/Bottleneck/ for better performance")
    bottleneck=np

this would use the numpy partition that is not as efficient but works on any numpy machine. I'm surprise the test passes.

[mzalaya]

good suggestion to sent a warning for non installed Bottelenck module.

[rflamary]

Hello @mzalaya,

So now autograd breaks on python 2.7. While we could deactivate the tests on 2.7 (it is deprecated) is would like to force to use an older version of pymanopt for the tests.

for that i think we can change the line pymanopt in requirement.txt and replace it with the two following lines:

pymanopt== 0.2.4, python_version < '3'
pymanopt, python_version >= '3'

Could you try to do that please? We will remove support for 2.7 at a future date but this is another PR ;)

[mzalaya]

@rflamary
a semicolon is needed between. cool now it's working :)
pymanopt==0.2.4; python_version <'3'
pymanopt; python_version >= '3'