Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to use QAOA-CV optimizer inside classification module #312

Merged
merged 57 commits into from
Sep 30, 2024

Conversation

gcattan
Copy link
Collaborator

@gcattan gcattan commented Sep 17, 2024

This PR allows to use QAOA-CV docplex optimizer inside the classification module.
To date, only naive QAOA optimizer and classical optimization are available.

Issues: #29, #30

@gcattan gcattan force-pushed the feat/qaoa-cv-classification branch from fb82bb1 to fcad45b Compare September 19, 2024 20:09
Comment on lines 226 to 227
"""Helper to create a docplex representation of a
covariance matrix variable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general remark for all dosctrings of pyRiemann-qiskit:
the short summary must be a one-line summary; you can give more details in the extended summary.

pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved

Parameters
----------
covmat : ndarray, shape (n_features, n_features)
Copy link
Member

@qbarthelemy qbarthelemy Sep 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two general remarks.

Like in pyRiemann, try to replace inputs called covmat by X, more sklearnistic.

And try to generalize description from covariance matrices to SPD matrices.

pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/docplex.py Outdated Show resolved Hide resolved
pyriemann_qiskit/utils/hyper_params_factory.py Outdated Show resolved Hide resolved
Comment on lines 34 to 35
import sys
import warnings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys and warnings are standard libraries, and must be imported in the first block of imports.

Copy link
Member

@qbarthelemy qbarthelemy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx @gcattan !

@gcattan
Copy link
Collaborator Author

gcattan commented Sep 30, 2024

Thank you for the review :)

@gcattan gcattan merged commit 05c68c4 into pyRiemann:main Sep 30, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants