Skip to content

Handle small-sample cross-fitting folds gracefully - #152

Merged
TomeHirata merged 1 commit into
mainfrom
ccr-e30220fe-ga9flf
Oct 6, 2026
Merged

TomeHirata merged 1 commit into
mainfrom
ccr-e30220fe-ga9flf

Conversation

@TomeHirata

@TomeHirata TomeHirata commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to the JOSS review's minor point on empty folds (openjournals/joss-reviews#11047):

#150 added a check for one case only: a fold leaving no target-arm training data. Most small-sample failures took other paths and still surfaced as opaque errors.

Changes (dte_adj/stratified.py, dte_adj/util.py)

  • Empty prediction folds are skipped. A fold (or a fold × stratum cell) with no records used to call model.predict_proba on an empty array. sklearn then raised Found array with 0 sample(s). Such a cell has nothing to predict, so it is now skipped. This was the most common failure, and those runs now simply succeed.
  • Informative errors for genuinely untrainable cells. The upfront check now also covers:
    • a fold × stratum cell with no target-arm training observations. This used to raise IndexError: index 0 is out of bounds. The error now names the fold and stratum and suggests reducing folds or merging small strata.
    • a stratum with no observations of the target arm at all.
  • Multi-task bug fix. When a fold × stratum cell's training labels were all identical, no model was fit for it. The code then reused the model from the previous cell, which was fit on another fold or stratum. If no model existed yet, it raised AttributeError: ... has no attribute 'model'. It now predicts the constant label, as the single-task path already does.

Effect (200 random seeds each, LogisticRegression, single-task)

Setting Before After
No strata, n=30, 10 folds 119 opaque errors 0 errors
No strata, n=20, 5 folds 25 opaque + 1 informative 1 informative
3 strata, n=20, 5 folds 200 opaque (IndexError / sklearn) 200 informative

With multi-task (LinearRegression), 3 strata, n=20 and 5 folds, 24 runs used to fail with AttributeError and 134 with the informative error. After the change, all 146 failures are informative. Failures can still happen at sizes this small. The difference is that each one now tells the user what went wrong and what to do.

Testing

  • New tests: four in TestSmallSampleFolds, covering an empty prediction fold, a stratum without training data, a stratum without the target arm, and multi-task constant labels. All four fail on main and pass with this change.
  • Full suite: 83 passed. ruff check . is clean.

- Skip folds (or fold x stratum cells) with nothing to predict instead of
  calling the model on an empty array, which raised an opaque sklearn
  'Found array with 0 sample(s)' error.
- Check up front that every fold x stratum cell has target-arm training
  data and that every stratum contains the target arm; raise a ValueError
  that names the fold/stratum and suggests reducing folds or merging strata
  instead of an IndexError.
- Multi-task: when a cell's training labels are constant, predict that
  constant rather than reusing a model fit on another fold/stratum (or
  failing with AttributeError when none exists yet).

Follow-up to the JOSS review minor point on empty folds
(openjournals/joss-reviews#11047).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjhUSYBaHyrPdnQ7z13FKv
Copilot AI balanced review requested due to automatic review settings October 6, 2026 12:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@TomeHirata
TomeHirata merged commit 684b37e into main Oct 6, 2026
10 checks passed
@TomeHirata
TomeHirata deleted the ccr-e30220fe-ga9flf branch October 6, 2026 12:31
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.

3 participants