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

Improvement of NCH #255

Merged
merged 18 commits into from
Mar 22, 2024
Merged

Improvement of NCH #255

merged 18 commits into from
Mar 22, 2024

Conversation

qbarthelemy
Copy link
Member

@qbarthelemy qbarthelemy commented Mar 20, 2024

This PR:

@qbarthelemy qbarthelemy requested review from gcattan and toncho11 March 20, 2024 15:34
@gcattan
Copy link
Collaborator

gcattan commented Mar 20, 2024

@qbarthelemy Can you complete the light benchmark, and check the scores after and before the 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.

Example file classify_P300_nch.py needs to be updated with the new parameter name subsampling and validated that it runs (python errors) and it completes correctly.

pyriemann_qiskit/classification.py Show resolved Hide resolved
@toncho11
Copy link
Collaborator

Currently I have this during the tests:

FAILED tests/test_utils_distance.py::test_qdistance_logeuclid_to_convex_hull[optimizer0] - assert 8.567767282575056e-07 == 0 ± 1.0e-12
FAILED tests/test_utils_distance.py::test_qdistance_logeuclid_to_convex_hull[optimizer1] - assert 8.567767282575056e-07 == 0 ± 1.0e-12

@qbarthelemy
Copy link
Member Author

Currently I have this during the tests:

FAILED tests/test_utils_distance.py::test_qdistance_logeuclid_to_convex_hull[optimizer0] - assert 8.567767282575056e-07 == 0 ± 1.0e-12 FAILED tests/test_utils_distance.py::test_qdistance_logeuclid_to_convex_hull[optimizer1] - assert 8.567767282575056e-07 == 0 ± 1.0e-12

Have you update/pull my branch with my last commits?

gcattan added 3 commits March 21, 2024 21:53
Add NCH to light_benchmark
fix warning in Ci/Cd
fix hull_type not renamed to subsampling
@toncho11
Copy link
Collaborator

toncho11 commented Mar 22, 2024

Currently I have this during the tests:
FAILED tests/test_utils_distance.py::test_qdistance_logeuclid_to_convex_hull[optimizer0] - assert 8.567767282575056e-07 == 0 ± 1.0e-12 FAILED tests/test_utils_distance.py::test_qdistance_logeuclid_to_convex_hull[optimizer1] - assert 8.567767282575056e-07 == 0 ± 1.0e-12

Have you update/pull my branch with my last commits?

I do:
git pull origin pull/255/head and I see the last changes from @gcattan coming
pip install .[docs] or pip install .[tests]
pytest
and I still get it:

>       assert dist == pytest.approx(0)
E       assert 8.567767282575056e-07 == 0 ± 1.0e-12
E         comparison failed
E         Obtained: 8.567767282575056e-07
E         Expected: 0 ± 1.0e-12

toncho11 and others added 3 commits March 22, 2024 11:52
Fixes tests by adjusting the precision.
Improved comment on n_samples_per_hull = -1
@gcattan
Copy link
Collaborator

gcattan commented Mar 22, 2024

LGTM! @qbarthelemy @toncho11 thank you for your work!

@gcattan gcattan merged commit 66ae851 into pyRiemann:main Mar 22, 2024
13 checks passed
@qbarthelemy qbarthelemy deleted the nch branch March 22, 2024 12:29
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.

3 participants