giotto-ai / giotto-tda

A high-performance topological machine learning toolbox in Python
https://giotto-ai.github.io/gtda-docs
Other
845 stars 173 forks source link

Update gudhi bindings to latest changes on August 2020 #468

Closed reds-heig closed 4 years ago

reds-heig commented 4 years ago

Reference issues/PRs Supersedes #466. Partially addresses #307.

Types of changes

Description

This PR integrates latest changes on gudhi modules, it also add the c++ code for the new Collapser feature.

Latest version of Gudhi requires Eigen as a dependency, fortunately, Eigen is a header only library, and we where able to add it as a git submodule directly.

Also integrating changes from @ulupo of setup.py, azure-pipelines.yml and docker_scripts.sh

Screenshots (if appropriate)

Any other comments?

@ulupo I think that this PR should only update the Gudhi bindings, and in a different PR, we should integrate the Collapser bindings for python, what do you think ?

Checklist

CLAassistant commented 4 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: ulupo
:x: julian


julian seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

ulupo commented 4 years ago

@ulupo I think that this PR should only update the Gudhi bindings, and in a different PR, we should integrate the Collapser bindings for python, what do you think ?

Totally agree!

gtauzin commented 4 years ago

Is the plan to include the collapser as an invisible step in each of the persistence transformer? This seems to be such an amazing achievement from the GUDHI team, I am really looking forward to trying it out!

ulupo commented 4 years ago

@gtauzin as far as I understand it, the collapser only works on flag complexes, so we would presumably only offer it for VietorisRipsPersistence (and maybe FlagserPersistence when directed is False; WeakAlphaPersistence should be fast enough without it). I don't know about SparseRipsPersistence, but we could look into it.