Repository navigation
Fix EEGRegressor loss broadcasting for one target per trial - #1197
Merged
Merged
Conversation
…n self from fit A (batch,) target against a (batch, 1) prediction was broadcast to (batch, batch) by the criterion, so every prediction was pulled toward the batch mean target. Reshape the target in get_loss. EEGRegressor.fit also now returns self like EEGClassifier.fit.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1197 +/- ##
==========================================
+ Coverage 87.73% 87.75% +0.02%
==========================================
Files 149 149
Lines 17452 17457 +5
==========================================
+ Hits 15312 15320 +8
+ Misses 2140 2137 -3 🚀 New features to boost your workflow:
|
Collaborator
|
thank you @Arthur031221 🙏🏽 |
3 tasks done
Contributor
|
Thanks for picking this up, and sorry I only saw this now! I had a fix in progress too. While testing it I noticed that cropped training has the same problem when |
bruAristimunha
pushed a commit
that referenced
this pull request
Oct 4, 2026
In cropped mode, EEGRegressor.fit reshapes a 1-D numpy y to (n, 1), but CroppedLoss squeezed the time-averaged prediction to (batch,), so the criterion compared every prediction with every target of the batch. #1197 fixed the trialwise counterpart of this in EEGRegressor.get_loss. CroppedLoss now keeps the output dimension when the target is a 2-D tensor. 1-D targets, such as class labels and dataset regression targets, and Mixup's (y_a, y_b, lam) list are handled as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Users who train a trial-wise
EEGRegressoron a dataset with one target per trial (create_from_X_y, or windows with a metadata target such as age) got a model that learns almost nothing: the(batch, 1)prediction was compared with a(batch,)target, andMSELossbroadcast the pair to(batch, batch), pulling every prediction toward the batch mean target.fit(X, y)with numpy arrays was not affected because it already reshapesy.Closes #1180. The report and reproduction are by @raghav-rathi. The issue has had no maintainer reply and I found no linked branch or PR for it. If @raghav-rathi already has a fix in progress, I am glad to close this in favour of theirs.
Changes
EEGRegressor.get_lossreshapes a 1-D tensor target to(batch, 1)when the prediction has shape(batch, 1), then calls the parentget_loss. Other shapes (cropped outputs, multi-output targets, Mixup tuples) are passed through unchanged.EEGRegressor.fitnow returnsself, likeEEGClassifier.fit, sonet.fit(X, y).predict(X)works.docs/whats_new.rstentry under Bugs.Testing
Reproduction from the issue (ShallowFBCSPNet, 16 trials, seed 0), loss reported by
net.get_lossagainst the mean squared error per trial:before:
loss: 1.3908 per-trial mse: 1.8189after:
loss: 1.5123 per-trial mse: 1.5123Two regression tests in
test/unit_tests/test_eegneuralnet.py: the loss equals the per-trial MSE for dataset targets, andfitreturns the estimator. Withregressor.pyreverted to master both fail; with the fixpytest test/unit_tests/test_eegneuralnet.pygives 55 passed.pytest test/unit_tests/training test/unit_tests/test_util.py test/unit_tests/augmentation/test_base.py: 78 passed, 8 skipped, 1 failed (test_predict_trials, needsmoabb, which is not installed in my environment; it fails the same way on master).ruff checkon the two changed Python files reports 6 findings, none on the added lines (checked against the diff hunks).pre-commititself and the full suite were not run.docs/whats_new.rstupdated.Notes for reviewers
The reshape is done in
get_lossrather than in the dataset or iterator so that classification, cropped decoding and Mixup targets keep their current paths. Targets that are already(batch, 1)are untouched.