WIP: Add ATOL - Githubissues

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

WIP: Add ATOL #494

Closed gtauzin closed 4 months ago

gtauzin commented 4 years ago

Reference issues/PRs

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Description

Add ATOL.

Screenshots (if appropriate)

Any other comments?

Checklist

[x] I have read the guidelines for contributing.
[x] My code follows the code style of this project. I used flake8 to check my Python changes.
[x] My change requires a change to the documentation.
[x] I have updated the documentation accordingly.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed. I used pytest to check this on Python tests.

gtauzin commented 4 years ago

@ulupo I will comment the code and the appropriate reference. It's the most complex transformer I am going to push before the release.

ulupo commented 4 years ago

@gtauzin thanks!

lewtun commented 4 years ago

one small question: i find ATOL completely non-transparent as a name and more so if i was a casual user. is it not possible to pick something a bit clearer?

gtauzin commented 4 years ago

one small question: i find ATOL completely non-transparent as a name and more so if i was a casual user. is it not possible to pick something a bit clearer?

Totally agreed. Any suggestions?

ulupo commented 4 years ago

If I understood correctly, the algorithm works as follow:

Find a bunch (a fixed number n say) of "centroids" for the entire collection of persistence diagrams.
Centre Gaussians or Laplacians around each.
For each diagram, sum the evaluations of each Gaussian/Laplacian at all persistence pairs (seen as points on the plane). So for each diagram one gets n features.

Hoping this can give some ideas...

ulupo commented 4 years ago

Since step 1 is in a sense clustering all diagrams in the collection, we could go for something like DiagramClusterFeatures or even just CentroidFeatures. But to be the devil's advocate: @lewtun, does the name really have to mean much? Would we make the same argument about e.g. UMAP? What I definitely agree with is that the acronym is not great.

lewtun commented 4 years ago

i like CentroidFeatures a lot - it definitely captures the main gist of what ATOL is about!

regarding UMAP, i don't think this is a fair comparison because UMAP is a mega famous algorithm + library that stands on it's own. by contrast, a casual user of giotto-tda would have no idea what this ATOL transformer is about just from the name. i'm putting myself in the shoes of someone who doesn't want to read documentation or academic papers before deciding to test out a feature extractor 😄

CLAassistant commented 3 years ago

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.
3 out of 4 committers have signed the CLA.

:white_check_mark: lewtun
:white_check_mark: ulupo
:white_check_mark: wreise
:x: Guillaume Tauzin

Guillaume Tauzin 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.}