From 825a0532a0bd57f652465662a4cdb94d14803e0f Mon Sep 17 00:00:00 2001 From: ChrisW09 <50968720+ChrisW09@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:47:00 +0200 Subject: [PATCH] fix(core): report output_dim, not min_output_dim, when it is too small ``_resolve_output_bounds`` reported every floor violation against ``min_output_dim``, but ``lo`` is set to ``output_dim`` on the non-adaptive branch. So setting ``output_dim=0`` blamed a parameter the caller had not touched -- and one that is ignored entirely when ``adaptive`` is False: Preprocessor(numerical_method="ple", output_dim=0) -> min_output_dim must be >= 1, got 0. Fix: raise min_output_dim to at least the family minimum. Following that advice would not have helped. Families that pre-validate ``output_dim`` themselves (cubicspline, bspline) already reported it correctly, so the two halves of the package disagreed. Name the parameter that produced the value. ple output_dim=0 -> output_dim must be >= 1, got 0. cubicspline output_dim=0 -> output_dim must be >= 3 for the cubic spline basis, got 0 adaptive, min_output_dim=0 -> min_output_dim must be >= 1, got 0. Closes #38 Co-Authored-By: Claude Opus 5 (1M context) --- pretab/core/adaptive.py | 9 +++++-- tests/test_adaptive_resolution.py | 40 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/pretab/core/adaptive.py b/pretab/core/adaptive.py index 3015715..7a388ec 100644 --- a/pretab/core/adaptive.py +++ b/pretab/core/adaptive.py @@ -88,9 +88,14 @@ def _resolve_output_bounds( label = floor_label if floor_label is not None else str(floor) if lo < floor: + # ``lo`` is ``output_dim`` on the non-adaptive branch and whenever no + # explicit ``min_output_dim`` was supplied, so naming the parameter + # unconditionally pointed users at a knob they had not set -- and one + # that is ignored when ``adaptive`` is False. + name = "min_output_dim" if self.adaptive and min_req is not None else "output_dim" raise InvalidParamError( - f"min_output_dim must be >= {label}, got {lo}.\n" - "Fix: raise min_output_dim to at least the family minimum." + f"{name} must be >= {label}, got {lo}.\n" + f"Fix: raise {name} to at least the family minimum." ) if ceil is not None and hi > ceil: raise InvalidParamError( diff --git a/tests/test_adaptive_resolution.py b/tests/test_adaptive_resolution.py index 7a5a10f..835974a 100644 --- a/tests/test_adaptive_resolution.py +++ b/tests/test_adaptive_resolution.py @@ -254,3 +254,43 @@ def test_preprocessor_adaptive_rbf_within_window(frame): out = pre.fit_transform(X, y, return_array=True) assert isinstance(out, np.ndarray) assert 2 * 3 <= out.shape[1] <= 2 * 9 + + +# --------------------------------------------------------------------------- # +# The floor error must name the parameter the caller actually set. +# +# ``lo`` is ``output_dim`` on the non-adaptive branch, but the message always +# said "min_output_dim" -- a knob the user had not touched, and one that is +# ignored entirely when ``adaptive`` is False. +# --------------------------------------------------------------------------- # +def test_floor_error_names_output_dim_when_not_adaptive(): + from pretab.core.exceptions import InvalidParamError + + rng = np.random.default_rng(0) + frame = pd.DataFrame({"a": rng.normal(size=50)}) + + with pytest.raises(InvalidParamError, match="output_dim must be >= 1, got 0"): + Preprocessor(numerical_method="ple", output_dim=0).fit(frame, rng.normal(size=50)) + + +def test_floor_error_does_not_mention_min_output_dim_when_not_adaptive(): + from pretab.core.exceptions import InvalidParamError + + rng = np.random.default_rng(0) + frame = pd.DataFrame({"a": rng.normal(size=50)}) + + with pytest.raises(InvalidParamError) as excinfo: + Preprocessor(numerical_method="ple", output_dim=0).fit(frame, rng.normal(size=50)) + + assert "min_output_dim" not in str(excinfo.value) + + +def test_floor_error_names_min_output_dim_when_it_was_set(): + from pretab.core.exceptions import InvalidParamError + from pretab.transformers import PLETransformer + + rng = np.random.default_rng(0) + X = rng.normal(size=(50, 1)) + + with pytest.raises(InvalidParamError, match="min_output_dim must be >= 1"): + PLETransformer(output_dim=5, adaptive=True, min_output_dim=0).fit(X, rng.normal(size=50))