Replace ripser.py and robinhood submodules with giotto-ph dependency

[ulupo]

Following the release of giotto-ph v0.2.1, we should be able to confidently remove the ripser.py and robinhood submodules, as well as the bindings for GUDHI's edge collapser, and add instead giotto-ph as a Python dependency.

@MonkeyBreaker this might be a good time to double-check that all tests in test_ripser.py and test_collapser.py have been migrated to the giotto-ph project.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[ulupo]

@MonkeyBreaker is the eigen git submodule still needed? I have tried to remove it in 6bd5a7c in a naive way, but this breaks the simplex tree interface from GUDHI.

[MonkeyBreaker]

@MonkeyBreaker is the eigen git submodule still needed? I have tried to remove it in 6bd5a7c in a naive way, but this breaks the simplex tree interface from GUDHI.

@ulupo looking were it is used in our fork of gudhi-devel, Eigen is used in the following places:

• Euclidean_strong_witness_complex.h
• Flag_complex_edge_collapser.h (This is not compiled anymore we can discard it)
• Kd_tree_search.h

It seems that unfortunately we cannot remove for the moment eigen.
And also, it seems that from looking at the simplex_tree, it needs the Flag_complex_edge_collapser.h file to compile. So we cannot not remove from our gudhi-devel the collapser.

Otherwise I clone this fork on my machine and I was able to compile, and run all the test.

[ulupo]

Hey @MonkeyBreaker, thanks! I'm ok to leave eigen for now. Perhaps there can be another PR which also brings our fork of gudhi-devel more up-to-date with the main branch there. I'll let you complete the review for this PR and just to reiterate:

@MonkeyBreaker this might be a good time to double-check that all tests in test_ripser.py and test_collapser.py have been migrated to the giotto-ph project.

[MonkeyBreaker]

@MonkeyBreaker this might be a good time to double-check that all tests in test_ripser.py and test_collapser.py have been migrated to the giotto-ph project.

Concerning test_ripser, all test from gtda are in gph. But, concerning test_collapser, gtda has additional test for csr format, this was removed because redundant with coo support.

In summary, all test for ripser and collapser from gtda were migrated in gph.

Thank you @ulupo for your effort on migrating gtda to use gph !

[MonkeyBreaker]

Hey @MonkeyBreaker, thanks! I'm ok to leave eigen for now. Perhaps there can be another PR which also brings our fork of gudhi-devel more up-to-date with the main branch there.

Definitely, this unfortunately takes some time ... and could break things if gudhi added additional dependencies in the modules we already integrated.

[ulupo]

In summary, all test for ripser and collapser from gtda were migrated in gph.

Just what I wanted to hear! Ok, I'll wait for your formal approval and for the CI to pass before merging.

[MonkeyBreaker]

Looking at the rest of the library the mention of ripser are in :

• doc/release.rst
• examples/persistent_homology_graphs.ipynb
• gtda/homology/_utils.py
• gtda/homology/simplicial.py

In the example file you mention, both libraries, this is perfectly fine.
In the utils, ripser is when we want a vr filtration.
In simplicial, it is in reference when we use ripser for computing a vr filtration.

For 2 last files, this is internal behavior to the library and users are not exposed to this.

LGTM !

Concerning the CI, we need to move to Github Action to be more confident on what is going on.

[ulupo]

The Install Boost step is failing because of a (504) Gateway Timeout error... Maybe the Boost website is down?

[MonkeyBreaker]

The Install Boost step is failing because of a (504) Gateway Timeout error... Maybe the Boost website is down?

Weird because for others windows jobs it works. Maybe the environment where things run have some issues ?

[ulupo]

for others windows jobs it works

But the other jobs were launched a few days ago. I'll re-run everything.

[ulupo]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).