diff --git a/CHANGELOG.md b/CHANGELOG.md index b9ec462..861d644 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Behavior change:** the default (`random_state=None`) now draws fresh entropy on every `fit()` call. Previously, default initialization was silently seeded with a fixed value regardless of configuration, so repeated fits produced identical results without any way to request a different draw. Pass `random_state=` for reproducible runs (#109). - `recommend_config()`'s `missingness` parameter now warns (`UserWarning`) when passed anything other than the default `"auto"`, instead of silently ignoring it. Recommendations are still bucketed by `p` only — the Option A trade study's example recommendations are too sparse per (p-bucket, missingness) cell (23 points across 3 p-buckets x 4 missingness categories) to bucket on responsibly without shipping unreplicated values (#110, see also #111). - `defaults.py`'s docstring corrected the unsupported "`hp_va` is the dominant lever" claim (the trade study's own marginal sensitivity data doesn't support it) and now documents a real validation: replicated (n_reps=8, seeded) rank_mae for the shipped bucket configs is 28-58% lower than the library default across all three p-buckets, at a 0.4-3.8% cost in holdout RMSE (#111). +- `recommend_config()` now warns (`UserWarning`) when `p` or the `p/n` aspect ratio falls outside the Option A trade study's validated region (`p` up to 200, `p/n` up to 2.0). Buckets are keyed on `p` alone with no upper bound, so genomics-scale data (small cohorts, thousands of features) silently received the same config as a balanced 100x100 matrix and empirically recovered the wrong rank entirely; there's no shape-aware recommendation to fall back to yet, but callers are no longer handed an untested extrapolation without warning (#116). ## [0.3.0] - 2026-08-17 diff --git a/src/vbpca_py/defaults.py b/src/vbpca_py/defaults.py index 957721a..d84381a 100644 --- a/src/vbpca_py/defaults.py +++ b/src/vbpca_py/defaults.py @@ -35,6 +35,16 @@ Recommendations are bucketed by the feature count ``p`` — the study's primary axis of rank-recovery difficulty. +**Validated range (#116):** the regime grid behind these buckets only +covers ``p`` up to 200 and ``p/n`` up to 2.0 — ``n`` itself never enters +the bucketing decision, so nothing distinguishes a balanced 100x100 +matrix from a small-cohort, thousands-of-features genomics matrix +(``p/n`` of 50-1000x) once ``p`` exceeds the "large" bucket's threshold +of 70. Empirically the "large" bucket's config does not transfer to that +shape (wrong rank recovered entirely); ``recommend_config`` warns when +``p``/``p over n`` fall outside the validated region, but there's no +tuned alternative to fall back to yet. + The returned dict is intended to be splatted into the estimator, e.g.:: from vbpca_py import VBPCA, recommend_config @@ -87,6 +97,12 @@ _SMALLP_MAX_P = 30 _TRANS_MAX_P = 70 +# Widest values actually present in the Option A trade study's regime +# grid, analysis/trade_study -- recommendations for anything past these +# bounds extrapolate rather than interpolate. +_MAX_VALIDATED_P = 200 +_MAX_VALIDATED_P_OVER_N = 2.0 + def _bucket(p: int) -> str: """Return the feature-count bucket for ``p`` features.""" @@ -132,6 +148,18 @@ def recommend_config( Raises: ValueError: If ``n`` or ``p`` is not positive, or if ``priority`` is not a recognised preset. + + Warns: + UserWarning: If ``p`` or the ``p/n`` aspect ratio falls outside + what the Option A trade study's regime grid covered (``p`` up + to 200, ``p/n`` up to 2.0). Buckets are keyed on ``p`` alone + with no upper bound, so e.g. genomics-scale data (small + cohorts, thousands of features -- ``p/n`` of 50-1000x) gets + the same config as a balanced 100x100 matrix despite being + nowhere near the validated region; empirically this can pick + the wrong rank entirely (see #116). There's no shape-aware + recommendation to fall back to yet -- this only flags that + the one returned is an extrapolation, not a fix. """ if n <= 0 or p <= 0: msg = f"n and p must be positive; got n={n}, p={p}" @@ -148,6 +176,18 @@ def recommend_config( UserWarning, stacklevel=2, ) + if p > _MAX_VALIDATED_P or p / n > _MAX_VALIDATED_P_OVER_N: + warnings.warn( + f"recommend_config(n={n}, p={p}) falls outside the Option A " + f"trade study's validated region (p up to {_MAX_VALIDATED_P}, " + f"p/n up to {_MAX_VALIDATED_P_OVER_N}); the bucketed " + f"recommendation is an extrapolation and has been observed to " + f"pick the wrong rank at extreme p/n ratios (e.g. small-cohort " + f"genomics data). See " + f"https://github.com/yoavram-lab/VBPCApy/issues/116.", + UserWarning, + stacklevel=2, + ) cfg = dict(_BUCKET_CONFIGS[_bucket(p)]) diff --git a/tests/test_defaults.py b/tests/test_defaults.py index 7a9e4ee..d5e3507 100644 --- a/tests/test_defaults.py +++ b/tests/test_defaults.py @@ -54,6 +54,26 @@ def test_recommend_config_explicit_missingness_warns() -> None: recommend_config(n=100, p=20, missingness="mcar") +def test_recommend_config_warns_beyond_validated_p() -> None: + """p past the trade study's max (200) warns it's an extrapolation.""" + with pytest.warns(UserWarning, match="validated region"): + recommend_config(n=200, p=2000) + + +def test_recommend_config_warns_beyond_validated_aspect_ratio() -> None: + """p/n past the trade study's max (2.0) warns even if p itself is small.""" + with pytest.warns(UserWarning, match="validated region"): + recommend_config(n=30, p=100) + + +def test_recommend_config_within_validated_region_does_not_warn() -> None: + """p and p/n within the trade study's grid stay silent.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + recommend_config(n=100, p=200) + recommend_config(n=200, p=150) + + def test_recommended_config_fits() -> None: """A recommended config is accepted by the estimator and fits.""" rng = np.random.default_rng(0)