-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add Riemann Quantum Classifier #31
Conversation
add some tests to riemann quantum classifier
pyriemann_qiskit/classification.py
Outdated
nfilter : int (default: 1) | ||
The number of filter for the xDawnFilter. | ||
The number of components selected is 2 x nfilter. | ||
dim_red : TransformerMixin (default: NaiveDimRed(is_even=True)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PCA seems a better default choice, because NaiveDimRed can easily loose discriminant features.
pyriemann_qiskit/classification.py
Outdated
@@ -417,3 +422,192 @@ def predict(self, X): | |||
""" | |||
_, labels = self._predict(X) | |||
return self._map_0_1_to_classes(labels) | |||
|
|||
|
|||
class RiemannQuantumClassifier(BaseEstimator, ClassifierMixin, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure that the current name is adapted.
The method looks like a full pipeline with strong constraints (Xdawn, tangent space, dimension reduction), rather than a new generic classifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I will try to find something more appropriate.
pyriemann_qiskit/datasets/utils.py
Outdated
"""Return mne epochs with duration 1s. | ||
Only visual left and right trials are selected. | ||
Data are returned filtered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you complete the description of the task of the signal, and add the link towards MNE dataset page?
Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>
Thanks for the review! There is still a problem with the pipeline, using python 3.8. To be honest I am not sure where it comes from. |
Ok, I compared the package version with older builds and it seems that the scipy version changed 2 days ago. @toncho11 FYI |
Thx @gcattan ! |
Thanks for the review :) |
This PR implements #23.
The helper classifier which is implemented here will be used for CVGridSearch.
A couple of tests were added to
test_classification.py
and required to refactorize some of the methods into a dataset/utils module.(WIP: missing documentation of the dataset module, and error in the pipeline)