Mapper + GIC wrapper

[MathieuCarriere]

A pull request for adding a Python class that can compute both Mapper and GIC complexes

[mglisse]

Many tests are still failing with "AttributeError: module 'gudhi' has no attribute 'NGIComplex'". You should have access to the errors by clicking on "Details" next to the red crosses at the bottom of this page. The same applies to some of your other branches.

[MathieuCarriere]

Argh I can't tell why some of the checks fail, when I compile the source on my machine, all tests succeed

[mglisse]

@VincentRouvreau can you have a look at this error?

Starting container gudhi/ci_for_gudhi:latest Warning: No authentication provided, using CircleCI credentials for pulls from Docker Hub. image cache not found on this host, downloading gudhi/ci_for_gudhi:latest

Error response from daemon: unauthorized: authentication required

[mglisse]

The tests also fail on my machine. In particular, the following line appears for many examples

AttributeError: 'CoverComplex' object has no attribute 'set_verbose'

and there is an assertion error in test_cover_complex() where a vertex 4 appears in the first complex.

[MathieuCarriere]

oh wow ok it seems that I missed these ones

[MathieuCarriere]

apparemment sphinx ne passe pas mais je n'arrive pas à déceler le problème

[mglisse]

https://github.com/sphinx-doc/sphinx/issues/8880 (not your fault)

[mglisse]

CI shows errors on platforms that do not have cgal

ImportError while importing test module '/root/project/src/python/test/test_cover_complex.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
../../../src/python/test/test_cover_complex.py:11: in <module>
    from gudhi import NGIComplex
E   ImportError: cannot import name 'NGIComplex' from 'gudhi' (/root/project/build/src/python/gudhi/__init__.py)

Maybe the test should only be executed if some-cmake-condition, or there should be a fallback somewhere, I didn't look at all.

[MathieuCarriere]

Hi Marc, thanks for all the useful comments!! After discussing with Vincent and Hind at Porquerolles, we decided to write three different classes for Mapper, Graph Induced and Nerve complexes, and revert NGIComplex back to CoverComplex. The changes should be less destructive now hopefully. I also removed the distance matrix saving, since it is not clear that it is so efficient (as you pointed out), but more importantly, it frequently breaks my code (as I end up using the matrix even when I don't want it if I am not careful with names). I also took all of your comments into account (I hope so at least).

[mglisse]

Can you remind me what the status of the old CoverComplex is? Does it provide some functionality that the new classes do not, or do you consider it as deprecated and we only keep it for compatibility with old user code?

[MathieuCarriere]

So basically CoverComplex had two modes Nerve and GIC, which were already cythonized by Vincent. What is new in this PR is that I created two sklearn classes (NerveComplex and GraphInducedComplex) for those two modes using Vincent functions (so no new functionalities, just sklearn wrappers), and then I created a brand new class MapperComplex. So yes CoverComplex is still used.

[mglisse]

So yes CoverComplex is still used.

It is used as an implementation detail for the others, my question was more asking if we still mean for users to use it directly or if we should hide it a bit and recommend only the new classes. With something like RipsComplexPersistence, the old RipsComplex provides access to the complex, not just its persistence diagram, so it still serves a purpose.

I now found a relevant bit in the doc: "CoverComplex usually provides better automatic tuning of the resolution parameter" (the part above about handling input files directly does not seem important). Is there a particular reason for that? Is it an inherent weakness in the sklearn-like interface that it somehow makes it impossible to tune as well?

[MathieuCarriere]

OK I see what you mean. Indeed, I think the only reason to stick to CoverComplex is if you prefer coding in C++, otherwise the Python classes contain strictly more functions. The remark about better tuning is an old comment from previous versions of the code that does not apply anymore so I removed it.

[MathieuCarriere]

Thanks a lot for your review Marc!! I've just pushed a batch of changes according to your comments, let me know what you think.