Skip to content

Commit b8ecdf1

Browse files
authored
[BUG] fix _score_params + add tests + fix errors 'masked' by the 'try... except'-block (#237)
### Bug fix: `_score_params` passed parameters incorrectly `_score_params` called `experiment(**params)`, unpacking the dict as keyword arguments. `BaseExperiment.__call__` expects a single positional dict, so every call raised a `TypeError` that was silently swallowed by the bare `except` clause. All candidates received `error_score` (NaN) and `np.argmax` always picked index 0, making optimization results appear stable but entirely meaningless. Fixed by changing `experiment(**params)` to `experiment(params)`. The `except` clause now also emits a `warnings.warn` so caught exceptions are no longer silent. ### Fix: deterministic test parameters for `TSCOptCV` The corrected `_score_params` exposed non-determinism in `TSCOptCV.get_test_params`: the `"stratified"` DummyClassifier strategy produces random predictions, and the default CV used `shuffle=True` without a fixed seed. Together these caused `test_fit_idempotent` to fail intermittently. Fixed by replacing `"stratified"` with `"prior"` (deterministic) and pinning `cv=KFold(n_splits=2, shuffle=False)`. ### Enhancement: `_predict_proba` override for `TSCOptCV` `TSCOptCV` overrode `_predict` with a `refit=False` guard but left `_predict_proba` unoverridden. With `refit=False`, `predict` raised `RuntimeError` while `predict_proba` silently delegated to an unfitted estimator. Added the same guard to `_predict_proba`. ### Tests Added unit tests for `_score_params` covering correct dict passing, return type, error score fallback, and warning emission.
1 parent 7e049cb commit b8ecdf1

File tree

4 files changed

+168
-7
lines changed

4 files changed

+168
-7
lines changed

src/hyperactive/integrations/sktime/_classification.py

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,39 @@ class labels for fitting
258258

259259
return self
260260

261+
def _predict_proba(self, X):
262+
"""Predict class probabilities for sequences in X.
263+
264+
private _predict_proba containing the core logic, called from predict_proba
265+
266+
State required:
267+
Requires state to be "fitted".
268+
269+
Accesses in self:
270+
Fitted model attributes ending in "_"
271+
272+
Parameters
273+
----------
274+
X : guaranteed to be of a type in self.get_tag("X_inner_mtype")
275+
if self.get_tag("X_inner_mtype") = "numpy3D":
276+
3D np.ndarray of shape = [n_instances, n_dimensions, series_length]
277+
if self.get_tag("X_inner_mtype") = "nested_univ":
278+
pd.DataFrame with each column a dimension, each cell a pd.Series
279+
for list of other mtypes, see datatypes.SCITYPE_REGISTER
280+
for specifications, see examples/AA_datatypes_and_datasets.ipynb
281+
282+
Returns
283+
-------
284+
y : 2D array of shape [n_instances, n_classes] - predicted class probabilities
285+
"""
286+
if not self.refit:
287+
raise RuntimeError(
288+
f"In {self.__class__.__name__}, refit must be True to make predictions,"
289+
f" but found refit=False. If refit=False, {self.__class__.__name__} can"
290+
" be used only to tune hyper-parameters, as a parameter estimator."
291+
)
292+
return super()._predict_proba(X=X)
293+
261294
def _predict(self, X):
262295
"""Predict labels for sequences in X.
263296
@@ -317,15 +350,16 @@ def get_test_params(cls, parameter_set="default"):
317350

318351
params_gridsearch = {
319352
"estimator": DummyClassifier(),
353+
"cv": KFold(n_splits=2, shuffle=False),
320354
"optimizer": GridSearchSk(
321-
param_grid={"strategy": ["most_frequent", "stratified"]}
355+
param_grid={"strategy": ["most_frequent", "prior"]}
322356
),
323357
}
324358
params_randomsearch = {
325359
"estimator": DummyClassifier(),
326-
"cv": 2,
360+
"cv": KFold(n_splits=2, shuffle=False),
327361
"optimizer": RandomSearchSk(
328-
param_distributions={"strategy": ["most_frequent", "stratified"]},
362+
param_distributions={"strategy": ["most_frequent", "prior"]},
329363
),
330364
"scoring": accuracy_score,
331365
}

src/hyperactive/opt/_common.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Common functions used by multiple optimizers."""
22

3+
import warnings
4+
35
__all__ = ["_score_params"]
46

57

@@ -14,7 +16,11 @@ def _score_params(params, meta):
1416
error_score = meta["error_score"]
1517

1618
try:
17-
return float(experiment(**params))
18-
except Exception: # noqa: B904
19-
# Catch all exceptions and assign error_score
20-
return error_score
19+
return float(experiment(params))
20+
except Exception as e:
21+
warnings.warn(
22+
f"Experiment raised {type(e).__name__}: {e}. "
23+
f"Assigning error_score={error_score}.",
24+
stacklevel=2,
25+
)
26+
return float(error_score)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""Tests for the opt module."""
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
"""Tests for _score_params to guard against parameter passing regressions."""
2+
3+
import numpy as np
4+
import pytest
5+
6+
from hyperactive.opt._common import _score_params
7+
8+
9+
class _DictExperiment:
10+
"""Minimal experiment stub that expects params as a single dict."""
11+
12+
def __call__(self, params):
13+
return params["x"] ** 2 + params["y"] ** 2
14+
15+
16+
class _DictOnlyExperiment:
17+
"""Experiment stub that rejects keyword arguments.
18+
19+
Fails loudly if params are passed as **kwargs instead of a dict,
20+
directly guarding against the ``experiment(**params)`` bug.
21+
"""
22+
23+
def __call__(self, params):
24+
if not isinstance(params, dict):
25+
raise TypeError(
26+
f"Expected a dict, got {type(params).__name__}. "
27+
"Parameters must be passed as a single dict, not as **kwargs."
28+
)
29+
return sum(v**2 for v in params.values())
30+
31+
32+
def _make_meta(experiment, error_score=np.nan):
33+
return {"experiment": experiment, "error_score": error_score}
34+
35+
36+
class TestScoreParams:
37+
"""Tests for the _score_params helper function."""
38+
39+
def test_params_passed_as_dict(self):
40+
"""Params must be passed as a single dict, not unpacked as **kwargs."""
41+
exp = _DictOnlyExperiment()
42+
meta = _make_meta(exp)
43+
params = {"x": 3.0, "y": 4.0}
44+
45+
score = _score_params(params, meta)
46+
47+
assert score == 25.0
48+
49+
def test_returns_correct_score(self):
50+
"""Score must match the experiment's return value."""
51+
exp = _DictExperiment()
52+
meta = _make_meta(exp)
53+
54+
assert _score_params({"x": 0.0, "y": 0.0}, meta) == 0.0
55+
assert _score_params({"x": 1.0, "y": 0.0}, meta) == 1.0
56+
assert _score_params({"x": 3.0, "y": 4.0}, meta) == 25.0
57+
58+
def test_returns_python_float(self):
59+
"""Return type must be a Python float, not numpy scalar."""
60+
exp = _DictExperiment()
61+
meta = _make_meta(exp)
62+
63+
result = _score_params({"x": 1.0, "y": 1.0}, meta)
64+
assert type(result) is float
65+
66+
def test_error_score_on_exception(self):
67+
"""When the experiment raises, error_score must be returned."""
68+
69+
def _failing_experiment(params):
70+
raise ValueError("intentional failure")
71+
72+
meta = _make_meta(_failing_experiment, error_score=-999.0)
73+
74+
with pytest.warns(match="intentional failure"):
75+
result = _score_params({"x": 1.0}, meta)
76+
77+
assert result == -999.0
78+
79+
def test_error_score_emits_warning(self):
80+
"""A caught exception must produce a warning, never be silent."""
81+
82+
def _failing_experiment(params):
83+
raise RuntimeError("boom")
84+
85+
meta = _make_meta(_failing_experiment, error_score=np.nan)
86+
87+
with pytest.warns(match="RuntimeError"):
88+
_score_params({"x": 1.0}, meta)
89+
90+
def test_many_params_passed_as_dict(self):
91+
"""Regression: many keys must not be unpacked as keyword arguments.
92+
93+
With the old ``experiment(**params)`` bug, this would raise
94+
TypeError inside __call__ because it only accepts one argument.
95+
"""
96+
97+
def _sum_experiment(params):
98+
return sum(params.values())
99+
100+
meta = _make_meta(_sum_experiment)
101+
params = {f"x{i}": float(i) for i in range(20)}
102+
103+
score = _score_params(params, meta)
104+
105+
assert score == float(sum(range(20)))
106+
107+
def test_with_base_experiment(self):
108+
"""Integration: works with a real BaseExperiment subclass."""
109+
from hyperactive.experiment.bench import Sphere
110+
111+
exp = Sphere(n_dim=2)
112+
meta = _make_meta(exp)
113+
114+
# Sphere minimum is at origin, value = 0
115+
# __call__ returns sign-adjusted score (higher is better)
116+
# Sphere is lower-is-better, so score = -evaluate
117+
score_origin = _score_params({"x0": 0.0, "x1": 0.0}, meta)
118+
score_away = _score_params({"x0": 3.0, "x1": 4.0}, meta)
119+
120+
assert score_origin > score_away

0 commit comments

Comments
 (0)