Skip to content

[BUG] get_params/set_params/clone are silently broken for default-constructed estimators #410

Description

@ChrisW09

Describe the bug
For default-constructed estimators (no explicit config objects), the sklearn parameter API is silently broken (deeptab/models/base.py:258-358):

  • get_params() returns {} — it drops random_state entirely (the legacy branch only returns _config_kwargs + preprocessor kwargs), and observability_config is missing from both branches.
  • clone(model) consequently loses random_state (and any other constructor state).
  • set_params(layer_sizes=[8]) only updates the _config_kwargs shadow dict, which is never used to rebuild self.config (verified by grep: _config_kwargs is only read back by get_params and serialization). The model still trains with the old architecture — while get_params() echoes the new value back, so the API actively lies.
  • Unknown/invalid parameter names are silently accepted in both branches; sklearn requires a ValueError.

Net effect: GridSearchCV/RandomizedSearchCV over a default-constructed deeptab estimator runs to completion while never changing a single hyperparameter.

To Reproduce

from sklearn.base import clone
from deeptab.models import MLPClassifier

m = MLPClassifier(random_state=42)
print(m.get_params())                  # {}  (random_state gone)
print(clone(m).random_state)           # None

m.set_params(layer_sizes=[8])
print(m.config.layer_sizes)            # [256, 128, 32]  -- unchanged!
print(m.get_params())                  # {'layer_sizes': [8]}  -- but echoed back

m.set_params(totally_bogus_param=123)  # silently accepted, no ValueError

Expected behavior
get_params/set_params/clone must round-trip all constructor state and actually affect the next fit; invalid names must raise ValueError. The split-config path (explicit model_config=/trainer_config=) behaves much better — the legacy flat-kwargs path should either be wired to the real config or removed.

Screenshots
n/a

Desktop (please complete the following information):

  • OS: macOS (Darwin 25.5.0, arm64)
  • Python version: 3.11.15
  • deeptab Version: 2.0.0 (main @ 4e6a359)

Additional context
The sklearn contract tests in tests/test_sklearn_contract.py pass because they parametrize estimators with explicit config objects, which takes the (healthier) split-config branch — the default-constructor path is never exercised. Related: __getstate__ (base.py:371) nulls state["task_model"] but the attribute is _task_model, so pickling serializes the full Lightning module despite the documented intent in _mixins/serialization.py (filed separately in the robustness umbrella issue).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions