fix: warn when recommend_config extrapolates beyond validated p/n range - #117
Merged
Merged
Conversation
…ge (#116) recommend_config buckets solely on p, with no upper bound and no use of n at all -- p>70 always maps to the same "large" bucket config regardless of how large p gets or how small n is. The Option A trade study's regime grid only covers p up to 200 and p/n up to 2.0; anything past that is an untested extrapolation. Empirically this breaks down at genomics-scale aspect ratios (small cohort, thousands of features): n=30,p=2000 recovers rank 9 instead of the true rank 5 using the same config as a balanced 100x100 matrix. Warns (UserWarning) when p or p/n falls outside the validated region, mirroring the existing missingness warning from #110. Doesn't fix the underlying gap -- that needs the Option A trade study extended with an aspect-ratio axis and re-run, tracked in #116 -- but callers are no longer silently handed an untested config. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #116.
recommend_config(n, p)buckets purely onp(smallp<=30,trans<=70,large>70), with no upper bound and no use ofnat all --p>70always maps to the identical "large" bucket config no matter how largepgets or how smallnis.Why this matters
Found while extending pp-eigentest's trade study to genomics-representative data shapes (small cohort, thousands of features --
p/nof 50-1000x, common for bulk RNA-seq/microarray data). The Option A trade study's own regime grid (analysis/trade_study) only coverspup to 200 andp/nup to 2.0 -- computed directly from everyTRAINING_REGIMES/VALIDATION_REGIMESentry. Anything past that is silently extrapolated.Empirically this breaks down:
recommend_config(n=30, p=2000)returns the identical config asrecommend_config(n=100, p=100), but recovers rank 9 instead of the true rank 5 (vs. correct recovery at the balanced shape).Change
recommend_confignow warns (UserWarning) whenp > 200orp/n > 2.0, mirroring the existingmissingness != "auto"warning from #110. This doesn't fix the underlying gap -- that needs the Option A trade study extended with an aspect-ratio axis and re-run, a bigger effort tracked in #116 for later -- but callers are no longer silently hunted an untested extrapolation.Test plan
just ci(format, lint, mypy --strict, pytest --cov) passes,defaults.pyat 100% coveragep=200, warns pastp/n=2.0even whenpitself is small, silent within the validated region (boundary valuesp=200/p/n=2.0inclusive)🤖 Generated with Claude Code