Skip to content

Ensure equal sampling of age across cross validation folds #108

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.

Sign up for GitHub

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

Open
HippocampusGirl wants to merge 3 commits into
base: main
Choose a base branch
from
+9 −5
Open

Ensure equal sampling of age across cross validation folds #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3218ae
Ensure equal sampling of age across cv folds
HippocampusGirl Mar 9, 2026
2af7de4
Remove atlases from .dockerignore
HippocampusGirl Mar 9, 2026
038b8cf
Update .dockerignore for new directory structure
HippocampusGirl Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,5 @@ fmriprep.egg-info
.git/datalad
.git/objects

wonkyconn/data/atlases
wonkyconn/data/giga_connectome
wonkyconn/data/halfpipe
data/giga_connectome
data/halfpipe
9 changes: 7 additions & 2 deletions wonkyconn/features/age_sex_prediction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from sklearn.decomposition import PCA # type: ignore[import-not-found]
from sklearn.impute import SimpleImputer # type: ignore[import-not-found]
from sklearn.linear_model import LogisticRegression, Ridge # type: ignore[import-not-found]
from sklearn.model_selection import ShuffleSplit, StratifiedShuffleSplit, cross_validate # type: ignore[import-not-found]
from sklearn.model_selection import StratifiedShuffleSplit, cross_validate # type: ignore[import-not-found]
from sklearn.pipeline import Pipeline # type: ignore[import-not-found]
from sklearn.preprocessing import LabelEncoder, StandardScaler # type: ignore[import-not-found]

Expand Down Expand Up @@ -51,7 +51,12 @@ def training_pipeline(
else:
y_train = np.asarray(target_labels)
estimator = Ridge(alpha=1.0)
cv_strategy = ShuffleSplit(n_splits=n_splits, test_size=0.2, random_state=random_state)

bins = pd.qcut(y_train, q=5, labels=False, duplicates="drop")
cv_strategy = StratifiedShuffleSplit(n_splits=n_splits, test_size=0.2, random_state=random_state)
splits = list(cv_strategy.split(np.zeros_like(bins), bins))

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using np.zeros_like(bins) as a dummy X is non-obvious, and it depends on bins shape/dtype semantics. Prefer a clearer placeholder that only encodes length (e.g., np.empty((len(bins), 1))), and/or add a short comment explaining that X is unused and bins supplies the stratification labels.

Suggested change
splits = list(cv_strategy.split(np.zeros_like(bins), bins))
# X is unused; we only need the correct number of samples for the splitter.
splits = list(cv_strategy.split(np.empty((len(bins), 1)), bins))

Copilot uses AI. Check for mistakes.
cv_strategy = splits
Comment on lines +55 to +58

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StratifiedShuffleSplit can raise at runtime if the binned target has too few samples in one or more bins (e.g., a bin count < 2, or only a single unique bin after duplicates="drop"). Consider adding a guard/fallback (e.g., dynamically reduce q based on bin counts/unique values, or fall back to non-stratified splitting when stratification isn’t feasible) so CV doesn’t fail on small or low-variance datasets.

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +58

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cv_strategy is used for two different concepts (first a splitter instance, then a concrete list of index splits). This makes the code harder to read/debug. Consider using distinct names like cv_splitter and cv_splits (and pass cv_splits into cross_validate) to keep intent clear.

Copilot uses AI. Check for mistakes.

scoring_metrics = {"mae": "neg_mean_absolute_error", "r2": "r2"}

pipe = Pipeline(
Expand Down
Loading