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.

Sign up for GitHub

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

Jump to bottom

Fix/svc kernel #220

Merged
merged 20 commits into from
Dec 8, 2023
Merged

Fix/svc kernel #220

merged 20 commits into from
Dec 8, 2023

Conversation

gcattan
Copy link
Collaborator

@gcattan gcattan commented Dec 6, 2023

This PR fixes the pipeline with Quantum SVM:

  • replace ZZFeatureMap with XFeatureMap
  • use predict_proba + softmax rather than predict
  • add missing test for multiclass classification.

Gregoire Cattan and others added 8 commits December 6, 2023 12:31
qbarthelemy
qbarthelemy requested changes Dec 7, 2023
View reviewed changes
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.

It's ok for me, but documentation build failed

https://github.com/pyRiemann/pyRiemann-qiskit/actions/runs/7119962622/job/19386188606?pr=220#step:5:2016

toncho11
toncho11 approved these changes Dec 8, 2023
View reviewed changes
Copy link
Collaborator

@toncho11 toncho11 left a comment

Choose a reason for hiding this comment

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

Looks good!
I added some notes on the comments.

pyriemann_qiskit/pipelines.py Show resolved Hide resolved
tests/test_classification.py Outdated Show resolved Hide resolved
tests/test_classification.py Outdated Show resolved Hide resolved
tests/test_classification.py Outdated Show resolved Hide resolved
pyriemann_qiskit/pipelines.py Outdated Show resolved Hide resolved
pyriemann_qiskit/pipelines.py Outdated Show resolved Hide resolved
pyriemann_qiskit/classification.py Outdated Show resolved Hide resolved
pyriemann_qiskit/classification.py Outdated Show resolved Hide resolved
examples/MI/plot_compare_dim_red.py Outdated Show resolved Hide resolved
@gcattan
Copy link
Collaborator Author

gcattan commented Dec 8, 2023 •
edited
Loading

Thank you both for the review :)

This bug was hard to find, and I will try to proceed (in next PRs) with the following improvement of our Ci:

  • Put more examples to run on Ci (not possible before, but might be possible with the new simulator)
  • Put the remaining examples to run before each release, or maybe every two weeks (following @toncho11 's suggestion)
  • Create a very very simple benchmark script to run with each PR.

@gcattan gcattan merged commit 7deee15 into pyRiemann:main Dec 8, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toncho11 toncho11 toncho11 approved these changes

@qbarthelemy qbarthelemy qbarthelemy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@gcattan @toncho11 @qbarthelemy