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

Added an example that uses datasets from MOABB and the RG+QuantumSVM pipeline #39

Merged
merged 45 commits into from
May 16, 2022
Merged

Added an example that uses datasets from MOABB and the RG+QuantumSVM pipeline #39

merged 45 commits into from
May 16, 2022

Conversation

toncho11
Copy link
Collaborator

The example visualizes and compares with two other "standard" pipelines.
It depends on the newest version of MOABB 0.4.6.

Copy link
Collaborator

@gcattan gcattan left a comment

Choose a reason for hiding this comment

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

Good job :) Can you in addition, add this example to doc/index.rst?

image

Comment on lines 56 to 66
class Vectorizer(BaseEstimator, TransformerMixin):
def __init__(self):
pass

def fit(self, X, y):
"""fit."""
return self

def transform(self, X):
"""transform. """
return np.reshape(X, (X.shape[0], -1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we intend to reuse this, can you add it to utils.filtering?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this since you changed your example?

shots=None, #'None' forces classic SVM
nfilter=2, #default 2
dim_red=PCA(n_components=10), #default 10, higher values render better performance with the SVM version used in qiskit
#q_account_token="" #IBM Quantum TOKEN
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you raise a defect for the q_account_token, following your observations in our Skype conversation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Follow-up opened here: #40

pipelines["Xdw+LDA"] = make_pipeline(
Xdawn(nfilter=2, estimator="scm"),
Vectorizer(),
LDA(solver="lsqr", shrinkage="auto")
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not have the same number of component in your feature vector, as compared with the two other pipelines. Is this intended?

ax.set_ylabel("ROC AUC")
ax.set_ylim(0.3, 1)

fig.show()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you post on a comment here the resulting figure?

from pyriemann_qiskit.classification import QuantumClassifierWithDefaultRiemannianPipeline
from sklearn.decomposition import PCA


This comment was marked as outdated.

Comment on lines 6 to 9
It uses QuantumClassifierWithDefaultRiemannianPipeline. This pipeline uses
Riemannian Geometry and Tangent Space to generate features and a quantum SVM
classifier. It uses MOABB for the evaluation and compariosn with 2 other
standard pipelines.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adapt here text according to our Skype conversation.

# Here we provide some pipelines for comparison:

# Standard SVM on raw data
from sklearn import svm
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this at the beginning.

toncho11 and others added 3 commits April 14, 2022 11:30
The number of pipelines has been reduced to two. This way it will be more simple to compare results.
add `print(__doc__)` to example
pass

def fit(self, X, y):
"""fit."""
Copy link
Member

Choose a reason for hiding this comment

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

Can you add descriptions for Parameters and Returns, for fit() and transform() ?

class Vectorizer(BaseEstimator, TransformerMixin):
"""This is an auxiliary transformer that allows one to vectorize data
structures in a pipeline For instance, in the case of an X with dimensions
Nt x Nc x Ns, one might be interested in a new data structure with
Copy link
Member

Choose a reason for hiding this comment

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

Can you use the standard names for dimensions: n_samples, n_features, n_channels, etc.

examples/ERP/classify_P300_bi.py Outdated Show resolved Hide resolved
doc/requirements.txt Show resolved Hide resolved
examples/ERP/classify_P300_bi.py Outdated Show resolved Hide resolved
examples/ERP/classify_P300_bi.py Outdated Show resolved Hide resolved
@toncho11
Copy link
Collaborator Author

Hi,

I think everything is ready for this PR to be accepted.

@gcattan
Copy link
Collaborator

gcattan commented Apr 30, 2022

@sylvchev did you have a chance to review the last modification?

Change the way `q_account_token` changed (updated in last PR)
@gcattan
Copy link
Collaborator

gcattan commented May 14, 2022

@sylvchev

@sylvchev sylvchev merged commit 901feff into pyRiemann:main May 16, 2022
@gcattan
Copy link
Collaborator

gcattan commented May 24, 2022

@toncho11 thanks for taking care of this. Do you think we can reuse some elements of your presentation at nbt-berlin to improve our documentation?

@gcattan
Copy link
Collaborator

gcattan commented Jul 31, 2022

I was wondering how can provides an estimate of the chance level for the bi2012 dataset. Is there something we can reuse from moabb @sylvchev ?

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.

4 participants