Skip to content

Ensure equal sampling of age across cross validation folds#108

Open
HippocampusGirl wants to merge 3 commits into
mainfrom
age-stratified-cv
Open

Ensure equal sampling of age across cross validation folds#108
HippocampusGirl wants to merge 3 commits into
mainfrom
age-stratified-cv

Conversation

@HippocampusGirl

@HippocampusGirl HippocampusGirl commented Mar 9, 2026

Copy link
Copy Markdown
Member

We split age into five quantiles and then ensure that these are equally distributed across train and test using StratifiedShuffleSplit. This may fix unusually high variability of model scores in the OASIS dataset.

We split age into five quantiles and then ensure that these are equally
distributed across train and test using StratifiedShuffleSplit
Copilot AI review requested due to automatic review settings March 9, 2026 19:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adjusts cross-validation splitting for the age regression task to better balance the age distribution between train/test folds by stratifying on age quantile bins.

Changes:

  • Replaced ShuffleSplit with StratifiedShuffleSplit for the regression CV strategy.
  • Added quantile binning (pd.qcut) of the regression target to enable stratification.
  • Precomputed CV splits using the binned target labels.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +55 to +58
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))
cv_strategy = splits

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
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))
cv_strategy = splits

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.

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.
@codecov

codecov Bot commented Mar 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.91%. Comparing base (68c04a7) to head (038b8cf).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
wonkyconn/features/age_sex_prediction.py 20.00% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
- Coverage   54.03%   53.91%   -0.12%     
==========================================
  Files          26       26              
  Lines        1414     1417       +3     
==========================================
  Hits          764      764              
- Misses        650      653       +3     
Files with missing lines Coverage Δ
wonkyconn/features/age_sex_prediction.py 36.58% <20.00%> (-2.89%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants