NeuroTechX / NeuroTechX/moabb

[follow-up] Optionally wire transfer-learning splitter into CrossSubjectEvaluation

Open
#1,088 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
1.1k
Forks
264
Avg merge
1d 13m
Merged PRs (30d)
23

Description

Summary

Optional follow-up to #1077. The transfer-learning splitter is a standalone scikit-learn cross-validator and already works on its own (for train, test in CrossSubjectTransferSplitter(...).split(y, metadata)), so it needs no change here to be usable. This issue is only about the convenience of running it through CrossSubjectEvaluation, to reuse its caching, results DB, parallelism, and scoring. That path is currently blocked because the top-level splitter is hardcoded.

Not a prerequisite for the splitter PR; track separately.

Current behaviour

Every evaluation builds its top-level splitter in _create_splitter() and passes cv_class/cv_kwargs only to the inner cross-validator, not to the splitter itself:

  • WithinSessionEvaluation -> WithinSessionSplitter (evaluations.py:86)
  • CrossSessionEvaluation -> CrossSessionSplitter (evaluations.py:299)
  • CrossSubjectEvaluation -> CrossSubjectSplitter (evaluations.py:479)
  • WithinSubjectEvaluation -> WithinSubjectSplitter (evaluations.py:660)
def _create_splitter(self):
    ...
    cv_class, cv_kwargs = self._resolve_cv(default_class, default_kwargs)
    return CrossSubjectSplitter(
        cv_class=cv_class, random_state=self.random_state, **cv_kwargs
    )

And the runner does self.cv = self._create_splitter() (base.py:758-764), with no public hook to inject a splitter. So CrossSubjectEvaluation(cv_class=CrossSubjectTransferSplitter, ...) does not swap the splitter; cv_class is forwarded into CrossSubjectSplitter as its inner fold strategy, which is not what a transfer setup needs.

Proposal

Let CrossSubjectEvaluation accept and use a top-level splitter, keeping the scikit-learn contract (split(y, metadata) -> (train_idx, test_idx)) untouched so caching, scoring, and result handling stay as-is. Two options, smallest first:

  1. Allow passing a splitter instance. If cv (or a splitter= argument) is a BaseCrossValidator that yields (train_idx, test_idx) over metadata, use it directly in _create_splitter() instead of hardcoding CrossSubjectSplitter. This makes CrossSubjectTransferSplitter usable today and is additive.

  2. Let cv_class select the top-level splitter when it is a MOABB splitter (detected by its split(y, metadata) signature), falling back to current behaviour otherwise.

I lean towards option 1: it is the least surprising, does not change any default, and keeps the transfer splitter a plain cross-validator rather than a new evaluation engine. Background and the splitter sketch are in #1077.

Scope

  • Follow-up only; the splitter itself ships and runs standalone via #1077.
  • Only CrossSubjectEvaluation for the transfer-learning case to start.
  • No change to defaults or to the split contract.
  • Tests against FakeDataset, mirroring how CrossSubjectSplitter is tested.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in evaluations.py at CrossSubjectEvaluation._create_splitter() around line 479, then read base.py around lines 758-764 and the existing CrossSubjectSplitter tests. Compare how a splitter instance could flow through the evaluation while preserving caching, scoring, and the split contract. Done means the transfer-learning splitter works through CrossSubjectEvaluation in FakeDataset tests without changing defaults.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, scikit-learn
Domain
machine-learning, testing-qa
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.