Skip to content

[BUG] State and lifecycle defects: seeding, failed fits, trainer kwargs, device argument #452

Description

@ChrisW09

Describe the bug

A group of state-management defects found by exercising fit/predict lifecycles. All reproduced.


set_seed()/seed_context() are inert for fit(): every fit re-seeds with its own random_state (default 101)

Where: deeptab/models/_mixins/fit.py (286, 396-405)

fit() unconditionally calls set_seed(random_state) with random_state defaulting to 101, so the documented with seed_context(k): model.fit(...) / set_seed(k) workflow has no effect whatsoever — every seed produces the identical model.

Observed: seed_context(1) and seed_context(999) produce bit-identical predictions (np.array_equal(...) -> True). Same for bare set_seed(5) vs set_seed(6) before fit. Reason: _FitMixin.fit has random_state: int = 101 in its signature and then does if random_state is not None: set_seed(random_state), which re-seeds with 101 and discards whatever the caller seeded.

Expected: Either the caller's global seed is respected when no explicit seed was requested (i.e. only seed when self.random_state or an explicit fit(random_state=...) was supplied — random_state should default to None, not 101), or the documentation must stop presenting set_seed/seed_context as a way to control training. As written, the docs are wrong in three places: docs/core_concepts/training_and_evaluation.md:404-412 (with seed_context(42): model.fit(...)), deeptab/core/reproducibility.py:32-41 (module docstring, same example), and set_seed's own docstring claim that it seeds "the full training pipeline — data splitting, weight initialisation, dropout masks, and DataLoader shuffling". The practical damage is silent: anyone running a multi-seed variance study or seed-averaged ensemble via set_seed gets N identical models and reports zero seed variance.

Repro
import warnings; warnings.simplefilter('ignore')
import numpy as np, pandas as pd
from deeptab import seed_context
from deeptab.models import MLPRegressor

rng = np.random.default_rng(0)
X = pd.DataFrame(rng.standard_normal((60, 4)), columns=list('abcd'))
y = X.values @ rng.standard_normal(4)
FIT = dict(max_epochs=1, batch_size=16, accelerator='cpu', devices=1,
           enable_progress_bar=False, enable_model_summary=False)

p = {}
for k in (1, 999):
    with seed_context(k):
        m = MLPRegressor()          # no random_state -> user relies on seed_context
        m.fit(X, y, **FIT)
    p[k] = m.predict(X)
print('identical:', np.array_equal(p[1], p[999]))   # -> True

fit() that raises part-way leaves the estimator permanently corrupted while still reporting is_fitted_ == True

Where: deeptab/models/_mixins/fit.py (142-177, 445-469, 570)

_build_model overwrites n_features_in_/input_columns_/feature_names_in_ and replaces self._data_module before it can fail, and nothing rolls back or clears is_fitted_, so a failed second fit destroys the previously fitted model while the estimator keeps claiming to be fitted.

Observed: ```
fit raised: InvalidParameterError
is_fitted_: True n_features_in_: 9 num_feature_info: None
original X -> ColumnCountError Expected 9 feature column(s) (as seen during fit), but got 4.
new X2 -> ValueError columns are missing: {'c', 'd', 'a', 'b'}

The good model is unreachable: predicting on the data it was actually trained on is rejected by a schema check that now reflects the *failed* fit, and predicting on the new data hits the stale (old-schema) preprocessor. The same class of corruption is silent when the failure happens later — if `trainer.fit` raises (e.g. a loss that throws), `is_fitted_` stays `True` and `predict()` happily returns numbers from the new, effectively untrained network (verified: predictions changed, maxdiff 0.020, no error).

*Expected:* `fit()` should either be transactional (build into locals and commit `n_features_in_`, `input_columns_`, `feature_names_in_`, `_data_module`, `_task_model`, `_best_model_path` only on success) or set `self.is_fitted_ = False` before mutating anything, so a failed fit leaves a cleanly unfitted estimator rather than one that lies about being fitted and cannot predict on any input.

<details><summary>Repro</summary>

```python
import warnings; warnings.simplefilter('ignore')
import numpy as np, pandas as pd
from deeptab.models import MLPRegressor

rng = np.random.default_rng(0)
X  = pd.DataFrame(rng.standard_normal((60, 4)), columns=list('abcd'))
y  = X.values @ rng.standard_normal(4)
X2 = pd.DataFrame(rng.standard_normal((60, 9)), columns=[f'z{i}' for i in range(9)])
y2 = X2.values @ rng.standard_normal(9)
FIT = dict(max_epochs=1, batch_size=16, accelerator='cpu', devices=1,
           enable_progress_bar=False, enable_model_summary=False)

m = MLPRegressor(random_state=42); m.fit(X, y, **FIT)
m.predict(X)                       # works
try:
    m.fit(X2, y2, val_size=1.5, **FIT)   # any mid-fit failure will do
except Exception as e:
    print('fit raised:', type(e).__name__)
print('is_fitted_:', m.is_fitted_, 'n_features_in_:', m.n_features_in_,
      'num_feature_info:', m._data_module.num_feature_info)
for nm, XX in (('original X', X), ('new X2', X2)):
    try:
        m.predict(XX); print(nm, '-> ok')
    except Exception as e:
        print(nm, '->', type(e).__name__, str(e)[:70])

fit(**trainer_kwargs) crashes with TypeError when the documented callbacks= Lightning argument is passed

Where: deeptab/models/_mixins/fit.py (496-509)

pl.Trainer(...) is constructed with a hard-coded callbacks=[...] list followed by **trainer_kwargs, so any user-supplied callbacks= collides and raises TypeError, even though the docs explicitly tell users to pass callback settings through fit().

Observed: TypeError: lightning.pytorch.trainer.trainer.Trainer() got multiple values for keyword argument 'callbacks'. There is no other supported way to attach a callback (no callbacks field on TrainerConfig), so custom LR logging, Optuna pruning callbacks, gradient-accumulation schedulers, etc. are simply unreachable.

Expected: callbacks should be handled the same way logger already is on line 504 — popped from trainer_kwargs and merged with the built-in [EarlyStopping, ModelCheckpoint, ModelSummary] list. docs/core_concepts/config_system.md:230 states "Runtime options such as accelerator, devices, precision, gradient_clip_val, and logger/callback settings are Lightning trainer keyword arguments... Pass them to fit(...) when needed", and docs/core_concepts/training_and_evaluation.md:309 says any unlisted keyword is "Forwarded to Lightning's Trainer" — neither is true for callbacks.

Repro
import warnings; warnings.simplefilter('ignore')
import numpy as np, pandas as pd
from lightning.pytorch.callbacks import LearningRateMonitor
from deeptab.models import MLPRegressor

rng = np.random.default_rng(0)
X = pd.DataFrame(rng.standard_normal((60, 4)), columns=list('abcd'))
y = X.values @ rng.standard_normal(4)
MLPRegressor(random_state=1).fit(
    X, y, callbacks=[LearningRateMonitor()],
    max_epochs=1, batch_size=16, accelerator='cpu', devices=1,
    enable_progress_bar=False, enable_model_summary=False)

NODE/ENODE data-aware threshold initialisation runs on the validation set during Lightning's sanity check

Where: deeptab/nn/initialization.py (26-33 (with deeptab/nn/blocks/node.py:282-290, 347-405))

ODST/ODSTE initialise their NaN-valued feature_thresholds and log_temperatures from "the first batch", but the first batch the module ever sees is a sanity-check validation batch, so learnable parameters are initialised from held-out data and the fitted model depends on num_sanity_val_steps.

Observed: init batches (shape, training): [((12, 80), False), ((12, 208), False)] with n_val = 12, n_train = 48, batch_size = 16 — the initialising batch has exactly the validation-set row count and the module is in eval mode, i.e. it is the sanity-check validation batch, not a training batch. Consequently num_sanity_val_steps=0 vs 2 gives different fitted models: identical: False, maxdiff 0.391.

Expected: Data-aware initialisation must be driven by training data only (e.g. run it explicitly in TaskModel.setup('fit') / on_train_start over a training batch, or guard ModuleWithInit.__call__ with self.training). As written it is train/validation leakage into learnable parameters for every NODE and ENODE model (both DenseBlock/ODST and ENODEDenseBlock/ODSTE derive from ModuleWithInit), and it makes the fitted model silently depend on a Lightning debugging knob.

Repro
import warnings; warnings.simplefilter('ignore')
import numpy as np, pandas as pd
from deeptab.models import NODERegressor
import deeptab.nn.blocks.node as nodemod

rng = np.random.default_rng(0)
X = pd.DataFrame(rng.standard_normal((60, 4)), columns=list('abcd'))
y = X.values @ rng.standard_normal(4)
FIT = dict(max_epochs=1, batch_size=16, accelerator='cpu', devices=1,
           enable_progress_bar=False, enable_model_summary=False)

ev = []
orig = nodemod.ODST.initialize
nodemod.ODST.initialize = lambda self, x, eps=1e-6: (
    ev.append((tuple(x.shape), self.training)), orig(self, x, eps=eps))[1]
m = NODERegressor(random_state=42); m.fit(X, y, **FIT)
nodemod.ODST.initialize = orig
print('init batches (shape, training):', ev[:2])
print('n_val =', len(m._data_module.y_val), 'n_train =', len(m._data_module.y_train))

def run(n):
    q = NODERegressor(random_state=42)
    q.fit(X, y, num_sanity_val_steps=n, **FIT)
    return q.predict(X)
r0, r2 = run(0), run(2)
print('identical:', np.allclose(r0, r2), 'maxdiff', float(np.abs(r0-r2).max()))

fit(random_state=...) is silently ignored whenever the constructor set random_state

Where: deeptab/models/_mixins/fit.py (396-398)

if self.random_state is not None: random_state = self.random_state overrides an explicit fit(random_state=...) argument with no warning, even though fit's own docstring documents random_state as a normal parameter ("random_state : int, default=101 — RNG seed for reproducibility").

Observed: identical: Truerandom_state=999 and random_state=1 are both discarded in favour of the constructor's 42. (For contrast, when the constructor leaves it None, fit(random_state=7) vs fit(random_state=8) do differ, so the argument is otherwise live.)

Expected: The more specific, per-call argument should win over the constructor default (or, if the constructor is deliberately authoritative, fit() should warn/raise on a conflicting explicit value and the docstring should say so). Silently discarding a caller-supplied seed makes per-fit seed sweeps on a single estimator object return identical models.

Repro
import warnings; warnings.simplefilter('ignore')
import numpy as np, pandas as pd
from deeptab.models import MLPRegressor

rng = np.random.default_rng(0)
X = pd.DataFrame(rng.standard_normal((60, 4)), columns=list('abcd'))
y = X.values @ rng.standard_normal(4)
FIT = dict(max_epochs=1, batch_size=16, accelerator='cpu', devices=1,
           enable_progress_bar=False, enable_model_summary=False)

a = MLPRegressor(random_state=42); a.fit(X, y, random_state=999, **FIT)
b = MLPRegressor(random_state=42); b.fit(X, y, random_state=1,   **FIT)
print('identical:', np.array_equal(a.predict(X), b.predict(X)))   # -> True

The device argument of predict()/predict_proba() is a silent no-op that accepts any value

Where: deeptab/models/_mixins/predict.py (41-65 (contract); deeptab/models/regressor_base.py:227-268 and deeptab/models/classifier_base.py:333-441 (implementations))

predict(X, embeddings=None, device=None) documents device as "Device override for inference (e.g. "cpu" to force CPU)", but no implementation reads the parameter — inference always runs on whatever device self._trainer picked at fit time, and even a nonsense device string is accepted without error.

Observed: all three identical: True — including for the invalid device string, which raises nothing. Grepping the package confirms device is never referenced in either predict body. On this host the fit trainer resolves to mps:0, so a caller asking for device="cpu" to work around an MPS kernel bug gets MPS anyway with no diagnostic.

Expected: Either honour the parameter (move self._task_model to the requested device and run inference there, validating the device string) or remove it from the public signatures and the docstring. A documented knob that silently does nothing — and swallows typos — is worse than not offering it.

Repro
import warnings; warnings.simplefilter('ignore')
import numpy as np, pandas as pd
from deeptab.models import MLPRegressor

rng = np.random.default_rng(0)
X = pd.DataFrame(rng.standard_normal((60, 4)), columns=list('abcd'))
y = X.values @ rng.standard_normal(4)
m = MLPRegressor(random_state=42)
m.fit(X, y, max_epochs=1, batch_size=16, accelerator='cpu', devices=1,
      enable_progress_bar=False, enable_model_summary=False)
a = m.predict(X)
b = m.predict(X, device='cpu')
c = m.predict(X, device='this-is-not-a-device')
print('all three identical:', np.array_equal(a, b) and np.array_equal(a, c))

profile(dry_run=True) does not restore state it documents as "left unchanged"

Where: deeptab/core/inspection.py (515-523)

The teardown clears only _task_model, _built, _data_module and is_fitted_; it leaves classes_, n_features_in_, input_columns_, feature_names_in_, a now-fitted _preprocessor, and an _estimator that has been mutated from the architecture class into an instance.

Observed: ```
before: False None True
after : [0 1 2] 4 ['a', 'b', 'c', 'd'] estimator is class: False

The `profile` docstring (deeptab/core/inspection.py:343-346) states: "dry_run : bool, default=True — When True the temporary model is discarded after profiling so the estimator's state is left unchanged". A never-fitted classifier now advertises `classes_`, `n_features_in_`, `feature_names_in_` and `input_columns_`, which is exactly the attribute set third-party code (and sklearn's default `check_is_fitted`) uses to decide an estimator is fitted.

*Expected:* The `finally` block should also delete `classes_`, `n_features_in_`, `input_columns_`, `feature_names_in_`, restore `self._estimator` to `type(self)._model_cls`, and rebuild an unfitted `self._preprocessor` — i.e. snapshot the relevant attribute set before the dry-run build and restore it afterwards.

<details><summary>Repro</summary>

```python
import warnings; warnings.simplefilter('ignore')
import numpy as np, pandas as pd
from deeptab.models import MLPClassifier

rng = np.random.default_rng(0)
X  = pd.DataFrame(rng.standard_normal((60, 4)), columns=list('abcd'))
yc = rng.integers(0, 3, size=60)
c = MLPClassifier()
print('before:', hasattr(c, 'classes_'), getattr(c, 'n_features_in_', None),
      isinstance(c._estimator, type))
c.profile(X, yc, dry_run=True)
print('after :', getattr(c, 'classes_', None), c.n_features_in_,
      c.input_columns_, 'estimator is class:', isinstance(c._estimator, type))

Expected behavior
set_seed() should influence subsequent fits (or the docs should state that random_state always
wins); a fit that raises should leave the estimator in its previous state or clearly unfitted; the
documented callbacks= passthrough should work; and a device= argument that does nothing should
either work or not exist.

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
torch 2.9.1, lightning 2.6.5, scikit-learn 1.9.0, numpy 2.4.6. Found in a second-pass review of v2.0.0
(seven independent lenses, each finding adversarially re-verified by a second reviewer, then re-run by
hand). Distinct from the already-filed #409-#426.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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