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

Remove subsample default in OrthogonalProcrustesAlignment, improve tests #28

Merged
merged 8 commits into from
Jun 27, 2023

Conversation

stes
Copy link
Member

@stes stes commented Jun 26, 2023

Multiple tests failed internally and on this repo (e.g., in #20 , #25, #26) due to a numerical issue in OrthogonalProcrustesAlignment when a too small value for subsample is used.

This PR fixes this by setting the default to None, which means that all points available in the embedding are used for alignment. This is the numerically best option. In other cases, warnings are emitted, when e.g. the values of subsample is smaller than the number of points in the dataset, or when the number of subsample is smaller than 1000 (a reasonable default).

This PR also improves the tests, and removes the atol value of several tests, or at least decreases it slightly to avoid false negatives in the future.


Fix https://github.com/AdaptiveMotorControlLab/CEBRA-dev/pull/647
Fix https://github.com/AdaptiveMotorControlLab/CEBRA-dev/pull/645
Fix https://github.com/AdaptiveMotorControlLab/CEBRA-dev/issues/644

@stes stes requested a review from MMathisLab June 26, 2023 19:29
@stes stes self-assigned this Jun 26, 2023
@cla-bot cla-bot bot added the CLA signed label Jun 26, 2023
@github-actions
Copy link
Contributor

Docstring Coverage Report

Name Total Miss Cover Cover%
main.py 5 1 4 80%
config.py 6 4 2 33%
data/datatypes.py 5 2 3 60%
data/helper.py 6 1 5 83%
data/multi_session.py 12 5 7 58%
datasets/demo.py 8 3 5 62%
datasets/gaussian_mixture.py 3 2 1 33%
datasets/hippocampus.py 7 1 6 86%
datasets/save_dataset.py 4 1 3 75%
datasets/allen/single_session_ca.py 9 3 6 67%
distributions/discrete.py 9 2 7 78%
distributions/multisession.py 5 1 4 80%
integrations/sklearn/helpers.py 2 1 1 50%
integrations/sklearn/metrics.py 3 1 2 67%
integrations/sklearn/utils.py 6 1 5 83%
models/projector.py 12 9 3 25%
------------------------------------------ -------- ------- -------- ---------
TOTAL 396 38 358 90.4%
(45 of 61 files omitted due to complete coverage)

RESULT: PASSED (minimum: 90.0%, actual: 90.4%)

@stes stes force-pushed the stes/fix-alignment branch from 1ed378b to a6d2fd1 Compare June 26, 2023 20:17
@stes stes merged commit 96d3b0f into main Jun 27, 2023
@stes stes deleted the stes/fix-alignment branch June 27, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants