cosanlab / cosanlab/nltools

align(): n_features=None should resolve to the smallest subject's voxel count for ragged SRM input

Closed Beginner friendly
#540 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
130
Forks
48
Avg merge
6h 42m
Merged PRs (30d)
34

Description

`align(method='srm' | 'deterministic_srm')` resolves `n_features=None` to subject 0's voxel count (`nltools/algorithms/alignment/procrustes.py`, the `n_features = int(data[0].shape[0])` line). For ragged input, the case SRM exists for, that default is larger than the smallest subject's voxel count whenever subject 0 is not the smallest, and the fit raises (before #535 it raised an opaque broadcast error; after slice S-H it raises a clear message naming the subject). The user has to compute the minimum themselves.

Proposed change: resolve `None` to `min(x.shape[0] for x in data)`. It changes a user-facing default, so it needs its own decision and a migration row; found during the #535 review of the S-H slice.

Contributor guide

Open the contributing guide

Research direction

Read nltools/algorithms/alignment/procrustes.py at the n_features = int(data[0].shape[0]) line and trace the SRM and deterministic_srm handling for ragged input. Confirm the default behavior for subjects with different voxel counts, then check the project’s migration-note format. Done means the proposed default is resolved from the smallest subject and the user-facing change has its own migration row.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
machine-learning
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.