Add ComplexPolynomial

[gtauzin]

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

Adds ComplexPolynomials to diagrams.features

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.

[CLAassistant]

All committers have signed the CLA.

[gtauzin]

Note to self : Needs to add references to the original paper and documentation entry.

[gtauzin]

@wreise Can I ask you to try out the corrections I have made?

[ulupo]

@gtauzin it seems that maintainers are not allowed edits on this one, could you enable it so I can proceed with typo fixing and the like?

[gtauzin]

@ulupo done!

[ulupo]

Thank you! For now, I just did some house cleaning. Since there are some pending conversations with @wreise, I'm focusing on reviewing and merging TopologicalVector first.

[gtauzin]

As for the title of the transformer, do you have any suggestion @ulupo @wreise? I am fine with the current one but the paper's title suggest also ComplexVector.

[gtauzin]

The problem of having hyperparameter check for a parameter that can be either an int or a list of int seems like it cannot be solved easily (see unresolved comment). @ulupo What do you think?

[wreise]

As for the title of the transformer, do you have any suggestion @ulupo @wreise? I am fine with the current one but the paper's title suggest also ComplexVector.

I think the current one is better than ComplexVector. It's literally a complex polynomial representation.

[ulupo]

@wreise @gtauzin not the most constructive comment on the naming issue but I note that, modulo the fact that we are using complex numbers, this is basically https://scikit-learn.org/stable/modules/generated/sklearn.preprocessing.PolynomialFeatures.html with interaction_only=True. Would this be enough to grant something like InteractionPolynomialFeatures? And we sweep the "complex" aspect under the carpet? I agree that "ComplexVector" says too little, but arguably this returns a "polynomial" only in an abstract sense that not many users would appreciate IMO.

[wreise]

Would this be enough to grant something like InteractionPolynomialFeatures? And we sweep the "complex" aspect under the carpet? I agree that "ComplexVector" says too little, but arguably this returns a "polynomial" only in an abstract sense that not many users would appreciate IMO.

Oh, nice link, but i would advise against using the same name. I believe that PolynomialFeatures has a different interpretation of the input - it just raises the inputs to different powers. We compute (and return) the coefficients of the polynomial whose roots are the input. While you can map the output of PolynomialFeatures to the coefficients of that polynomial, I don't think this is enough to justify the link. Then, there is also the question of us applying one of the transformations (R, S, T), which is completely absent from PolynomialFeatures.

[ulupo]

Oh, nice link, but i would advise against using the same name. I believe that PolynomialFeatures has a different interpretation of the input - it just raises the inputs to different powers. We compute (and return) the coefficients of the polynomial whose roots are the input. While you can map the output of PolynomialFeatures to the coefficients of that polynomial, I don't think this is enough to justify the link.

I don't follow this, I still believe the link is as strong as initially claimed. But I'll check again when I have more time.

[ulupo]

Then, there is also the question of us applying one of the transformations (R, S, T), which is completely absent from PolynomialFeatures.

But I do agree with this, so I was only talking about the default behaviour.

[wreise]

I don't follow this, I still believe the link is as strong as initially claimed. But I'll check again when I have more time.

True, my bad, sorry. I misread the entry format.

[ulupo]

@gtauzin could you assist with the following:

• Fix the failing tests;
• Reinstate the possibility of using either list or int types following #502 .

[wreise]

Reinstate the possibility of using either list or int types following #502 .

Isn't it already possible? I think it's tested in gtda/diagrams/tests/test_features_representations.py:96.

[wreise]

Thanks @ulupo for the docstrings! They look better now. IMO, it's ready to be merged.

[ulupo]

Thanks @wreise, indeed I just have a couple of things left to commit in my machine and then we're really done here!