-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix probabilities returned by label_components #36
Conversation
Codecov Report
@@ Coverage Diff @@
## main #36 +/- ##
=======================================
Coverage 96.26% 96.26%
=======================================
Files 12 12
Lines 509 509
=======================================
Hits 490 490
Misses 19 19
Continue to review full report at Codecov.
|
@@ -56,7 +56,7 @@ def label_components(inst: Union[BaseRaw, BaseEpochs], ica: ICA, method: str): | |||
labels_pred_proba = methods[method](inst, ica) | |||
labels_pred = np.argmax(labels_pred_proba, axis=1) | |||
labels = [ICLABEL_NUMERICAL_TO_STRING[label] for label in labels_pred] | |||
y_pred_proba = labels_pred_proba[:, labels_pred] | |||
y_pred_proba = labels_pred_proba[np.arange(15), labels_pred] |
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.
Doesn't this mean predictions are always predicting 15 components? Even if ICA produces e.G. 30 components?
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.
oh my god.. yes..
I was doing too many things at once, I'll fix it when I get home in 20 minutes.
Previous selection would return a square matrix, e.g. if 15 components were provided, we would go from
(15, 7)
shape to(15, 15)
instead of(15, )
.